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

bpo-32267: Fix parsing tz offsets microseconds shorter than 6 #4781

Merged
merged 1 commit into from
Jan 9, 2018

Conversation

mariocj89
Copy link
Contributor

@mariocj89 mariocj89 commented Dec 10, 2017

As the remainder is directly parsed as an int, strings like .600 will be parsed as 600 microsecond rather than milliseconds.

I've added no news entries as it was included in #4015 and I think it has not been released yet. Please let me know if one should be added.

https://bugs.python.org/issue32267

As the remainder is directly parsed as an int, strings like
.600 will be parsed as 600 microsecond rather than miliseconds.
@mariocj89 mariocj89 changed the title Fix when parsing tz offsets microseconds shorter than 6 bpo-32267: Fix when parsing tz offsets microseconds shorter than 6 Dec 10, 2017
@mariocj89 mariocj89 changed the title bpo-32267: Fix when parsing tz offsets microseconds shorter than 6 bpo-32267: Fix parsing tz offsets microseconds shorter than 6 Dec 10, 2017
gmtoff_fraction = int(z[8:] or 0)
gmtoff_remainder = z[8:]
# Pad to always return microseconds.
gmtoff_remainder_padding = "0" * (6 - len(gmtoff_remainder))
Copy link
Member

Choose a reason for hiding this comment

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

I did a microbenchmark for this vs. multiplying by 10**(6-len(gmtoff_remainder)) and it seems like your version is either faster or the same speed. Maybe that's why you chose this method, but I thought I'd chime in since I went through the trouble to check it.

@mariocj89
Copy link
Contributor Author

@abalkin sorry to mention you but without this change the previous PR change is buggy.
Mentioning as probably as the commit status is failed it will fall off the radar. If you want I can create a news entry but I think it makes no sense

@pganssle
Copy link
Member

pganssle commented Jan 4, 2018

FWIW this patch looks good to me.

@abalkin abalkin self-assigned this Jan 9, 2018
@abalkin abalkin merged commit f80c0ca into python:master Jan 9, 2018
@mariocj89
Copy link
Contributor Author

Thanks!

@mariocj89 mariocj89 deleted the fix_strptime_us_isoparse branch September 14, 2019 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants