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

OPENNLP-1627 - Add new models from ud 1.1 familiy #14

Merged
merged 1 commit into from
Oct 17, 2024
Merged

Conversation

rzo1
Copy link
Contributor

@rzo1 rzo1 commented Oct 15, 2024

Wrote some local glue code to auto generate those new models because I didn't like to do copy paste...

So here it is ...

@rzo1 rzo1 marked this pull request as ready for review October 15, 2024 09:35
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

I didn't like to do copy paste...

👍👏

@atarora
Copy link

atarora commented Oct 15, 2024

Wow, This looks thorough! Great work ! <3

@mawiesne mawiesne changed the title OPENNLP-1627 - Add new models form ud 1.1 familiy OPENNLP-1627 - Add new models from ud 1.1 familiy Oct 15, 2024
Copy link
Contributor

@mawiesne mawiesne left a comment

Choose a reason for hiding this comment

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

I double checked:

  • treebank naming vs. lang code: correct in all cases.
  • full list of supported languages (README.md)

@mawiesne mawiesne self-requested a review October 17, 2024 11:31
@mawiesne
Copy link
Contributor

@rzo1 What do you make from README.md => "Ensure to add a new model to the expected-models.txt file located in opennlp-models-test."

Shouldn't this be included/change with this PR also?

@rzo1
Copy link
Contributor Author

rzo1 commented Oct 17, 2024

It is changed but GH doesn't show it in the web view, cf. 037cec7#diff-85b66f71480e436210ee3c949738f0ad822ceb95e38b851866d463a90525e137

@mawiesne
Copy link
Contributor

It is changed but GH doesn't show it in the web view, cf. 037cec7#diff-85b66f71480e436210ee3c949738f0ad822ceb95e38b851866d463a90525e137

Ic, well this seems to be just fine then.

@mawiesne mawiesne merged commit 1e2931b into main Oct 17, 2024
2 checks passed
@mawiesne mawiesne deleted the OPENNLP-1627 branch October 17, 2024 11:37
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.

4 participants