Catch infinite decimal values in _is_decimal (fix #662)#679
Conversation
| if not decimal_value.is_finite(): | ||
| raise ValueError("Converted decimal value is infinite or NaN") | ||
|
|
||
| return decimal_value |
There was a problem hiding this comment.
I think it's preferable to put the return statement outside the try/except block.
There was a problem hiding this comment.
@pganssle thanks for a fast review!
I think, it would be better to put it into else statement, meaning that the code
must be executed if the try clause does not raise an exception
Then we would not have variable referenced before assignment warning.
|
Can you add a change to the changelog? See CONTRIBUTING.md, you'll want to add Also, please add yourself to the |
|
Looks perfect! Will merge when tests pass. Thanks so much for the report and bugfix! |
|
@pganssle thank you for checking this out so quickly and for all your suggestions, that is very pleasant and important! |
|
@amureki Thanks for your kind words, and I'm glad you had a good experience contributing to dateutil! |
|
@pganssle hello! Would you, please, update me on the release process? How often do we do this, what are the prerequisites for it? I would love to see the new version released. :) |
|
Releases happen every once in a while when I have time to make one. I'm trying to get better about making frequent releases. I'd like to get #700 merged first, and generally wrap up everything in the 2.7.3 milestone. Additionally, I'm thinking that the already-merged #672 will actually be a performance regression without implementing #691, so I'm planning on creating a new 2.7.x branch for future patch releases. |
|
Oh, thank you for the detailed explanation of the release process! |
2 participants
This fixes an edge case for decimal parsing.
When we are receiving
Nanor infinite value in_to_decimal(which theoretically should not be possible), we are raising aValueError, instead of continuing with it.Thank you @pganssle for the suggestion, please, check if this solution fits well.