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

Jump to bottom

Except all errors when checking for valid unit #424

Merged
merged 5 commits into from Apr 28, 2022
Merged

Except all errors when checking for valid unit #424

merged 5 commits into from Apr 28, 2022

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Apr 26, 2022

I believe this is a good use case for not only catching specific exceptions.

@Flix6x
Add test case that lets pint throw an AttributeError
2f1b1f7 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Except all errors when checking for unit validity using pint
bc46d17 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x self-assigned this Apr 26, 2022
@Flix6x Flix6x added bug Something isn't working Units Deals with unit conversion labels Apr 26, 2022
@Flix6x Flix6x added this to the 0.9.4 milestone Apr 26, 2022
@Flix6x
Changelog entry
092fbe9 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x marked this pull request as ready for review April 26, 2022 14:31
@Flix6x Flix6x requested a review from nhoening April 26, 2022 14:31
@nhoening
Copy link
Contributor

Can you argue why this use case is different to the norm and deserves the noqa B902 ?

@coveralls
Copy link
Collaborator

coveralls commented Apr 26, 2022 •
edited

Pull Request Test Coverage Report for Build 2239847775

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 68.514%
Totals Coverage Status
Change from base Build 2219869932: 0.005%
Covered Lines: 7051
Relevant Lines: 9762
💛 - Coveralls

@Flix6x
Copy link
Contributor Author

Flix6x commented Apr 26, 2022

Because this is the third time I've encountered a different error thrown by pint when trying to parse a string to a Quantity. I could add the newly found AttributeError to the list explicitly, but what if we encounter even more types of errors later?

@nhoening
Copy link
Contributor

Is the code (docstring) of ur.Quantity helpful?

If not, and you want to stop worrying, then add a comment and move on, indeed.

@Flix6x
Add inline note
f6866ff 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x
Update patch release date
0d039cb 
Signed-off-by: F.N. Claessen <felix@seita.nl>
@Flix6x Flix6x merged commit db84393 into main Apr 28, 2022
@Flix6x Flix6x deleted the except-all-errors-when-checking-for-valid-unit branch April 28, 2022 14:12
Flix6x added a commit that referenced this pull request Apr 28, 2022
@Flix6x
Backport PR #424: Except all errors when checking for valid unit (#424)
1da3c48 
* Add test case that lets pint throw an AttributeError

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Except all errors when checking for unit validity using pint

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Changelog entry

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Add inline note

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Update patch release date

Signed-off-by: F.N. Claessen <felix@seita.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhoening nhoening Awaiting requested review from nhoening

Assignees

@Flix6x Flix6x

Labels
bug Something isn't working Units Deals with unit conversion
Projects
None yet
Milestone
0.9.4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Flix6x @nhoening @coveralls