Fix issue 427 #570
Fix issue 427 #570
Conversation
|
@m-dz Do you want to add yourself to the Authors file, or you can just comment here with the right name / e-mail (optional) to use? |
|
@pganssle , will do it, thanks! |
dateutil/parser/_parser.py
Outdated
| @@ -42,6 +42,8 @@ | |||
| import six | |||
| from six import binary_type, integer_types, text_type | |||
|
|
|||
| from decimal import Decimal, getcontext | |||
dateutil/parser/_parser.py
Outdated
|
|
||
| # See GH issue #427, fixing float rounding | ||
| # Conversion here might be redundant as value is already passed as Decimal() | ||
| # but _parse_min_sec() is also called in _parse_numeric_token where it is still a float(). |
There was a problem hiding this comment.
If it's already float, you're not making anything better by converting it to Decimal.
>>> from decimal import Decimal
>>> Decimal(5.6)
Decimal('5.5999999999999996447286321199499070644378662109375')The problem is to avoid converting it to a float in the first place.
There was a problem hiding this comment.
If this is not causing any problems just drop the Decimal call.
dateutil/test/test_parser.py
Outdated
| @@ -917,6 +917,18 @@ def test_pre_12_year_same_month(self): | |||
| dtstr = '0003-03-04' | |||
| assert parse(dtstr) == datetime(3, 3, 4) | |||
|
|
|||
| def test_rounding_float_like_strings(self): | |||
There was a problem hiding this comment.
This is a mix of pytest and unittest style tests. For the unittest style tests, you use self.assertEqual and the name would be testRoundingFloatLikeStrings. I'm fine with you de-indenting it so it's not in a class and use pytest, like so:
@pytest.mark .parametrize('dtstr,dt', [
('5.6h', datetime(2009, 9, 25, 5, 36)),
('5.6m', datetime(2009, 9, 25, 0, 5, 36)),
('5.6s', datetime(2009, 9, 25, 0, 0, 5, 600000))
]
def test_rounding_floatlike_strings(dtstr, dt):
assert parse(dtstr, default=datetime(2003, 9, 25)) == dtThough actually '5.6s' never had this problem (that's fine, but it might be worth a comment).
|
@pganssle , just saw your comments, will have a look in the afternoon. |
|
@pganssle , might be better, but I am still not sure where to put the tests. |
|
@m-dz I think where you put it is fine. Thanks for the PR and update! I'm not sure that these other |
|
@pganssle , thanks for help and useful comments! I have changed Ah, it was a pleasure! |
Fixes issue #427 with all tests passing