Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python script/samples updates #622

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Python script/samples updates #622

merged 1 commit into from
Feb 10, 2025

Conversation

Enkidu93
Copy link
Collaborator

@Enkidu93 Enkidu93 commented Feb 10, 2025

Fixes #621
Fixes #609

Changes:

  • Remove ServalApp
  • Update python client/download debug script
  • Fix README typos

This change is Reviewable

@Enkidu93 Enkidu93 requested a review from johnml1135 February 10, 2025 18:43
Copy link
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.50%. Comparing base (9743ae8) to head (7898569).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #622   +/-   ##
=======================================
  Coverage   64.50%   64.50%           
=======================================
  Files         280      280           
  Lines       14113    14113           
  Branches     1824     1824           
=======================================
  Hits         9103     9103           
  Misses       4349     4349           
  Partials      661      661           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@johnml1135
Copy link
Collaborator

scripts/download_build_data.py line 110 at r1 (raw file):

            pretranslation_objs += list(map(lambda p: p.to_jsonable(), pretranslations))

        # PARALLEL CORPORA

I am assuming you tested this out. If it works, I am fine with it.

Copy link
Collaborator

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93)

Copy link
Collaborator Author

@Enkidu93 Enkidu93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @johnml1135)


scripts/download_build_data.py line 110 at r1 (raw file):

Previously, johnml1135 (John Lambert) wrote…

I am assuming you tested this out. If it works, I am fine with it.

Yes, it works

Copy link
Collaborator

@johnml1135 johnml1135 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)

@Enkidu93 Enkidu93 merged commit c271f57 into main Feb 10, 2025
3 checks passed
@Enkidu93 Enkidu93 deleted the update_serval_python_client branch February 10, 2025 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove outdated ServalApp download_build_data script needs to be updated to accommodate ParallelCorpora
4 participants