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

more precise type hint for eval_on_train_fraction #2811

Merged
merged 2 commits into from
Jun 17, 2022
Merged

more precise type hint for eval_on_train_fraction #2811

merged 2 commits into from
Jun 17, 2022

Conversation

thlor
Copy link
Contributor

@thlor thlor commented Jun 15, 2022

The train() method of trainer.py accepts an eval_on_train_fraction parameter, with either a float or "dev" literal string value (see docstring). This commit adds the "dev" literal option to the type hint.

The train() method of trainer.py accepts an eval_on_train_fraction parameter, with either a float or "dev" literal string value (see docstring). This commit adds the "dev" literal option to the type hint.
@thlor
Copy link
Contributor Author

thlor commented Jun 15, 2022

Looks like Literal type hint is only available from python 3.8 but the tests run on 3.7. I guess we can get back to this when 3.7 support is dropped, and maybe add a plain str type hint in the meantime? At least this way IDEs won't complain about incorrect typing when passing "dev" as the argument (though won't complain about any other, invalid strings, either).

The train() method of trainer.py accepts an eval_on_train_fraction parameter, with either a float or "dev" literal string value (see docstring). This commit adds the string option to the type hint. Once python 3.7 is dropped, this can be changed to the even more precise "dev" Literal, see: c1791f0
@alanakbik
Copy link
Collaborator

@thlor thanks for adding this!

@alanakbik alanakbik merged commit 19fc60f into flairNLP:master Jun 17, 2022
@alanakbik alanakbik mentioned this pull request Jun 17, 2022
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.

2 participants