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

add test and update error message for functional.py::load_sp_model #984

Merged
merged 4 commits into from
Sep 16, 2020

Conversation

astaff
Copy link
Contributor

@astaff astaff commented Sep 16, 2020

This PR:

  • adds test for unsupported input type test_sentencepiece_unsupported_input_type (review comment)
  • updates error message listing valid input types and type of input given. (review comment)

Notes:

  • Should we raise TypeError instead of RuntimeError?
  • Type annotation and static type checking is another option to do that.

Copy link
Contributor

@zhangguanheng66 zhangguanheng66 left a comment

Choose a reason for hiding this comment

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

LGTM. Please sync with the master branch so the failed CI tests will go away.

@zhangguanheng66
Copy link
Contributor

I like TypeError.

@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #984 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #984      +/-   ##
==========================================
+ Coverage   77.67%   77.70%   +0.03%     
==========================================
  Files          44       44              
  Lines        3099     3099              
==========================================
+ Hits         2407     2408       +1     
+ Misses        692      691       -1     
Impacted Files Coverage Δ
torchtext/data/functional.py 89.47% <100.00%> (+2.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e8485b...9004f49. Read the comment docs.

@zhangguanheng66 zhangguanheng66 merged commit 0302ea9 into pytorch:master Sep 16, 2020
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.

None yet

3 participants