Skip to content

Fix mismatching parameter in code example of time.fromisoformat() #103246

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

Closed
wants to merge 0 commits into from
Closed

Fix mismatching parameter in code example of time.fromisoformat() #103246

wants to merge 0 commits into from

Conversation

mikelei8291
Copy link
Contributor

The example code was intended to show that the comma was a valid delimiter for fractional seconds, but the returned datetime.time object was not consistent with the specified parameter.

The example was added in #92177.

@@ -1786,7 +1786,7 @@ Other constructor:
datetime.time(4, 23, 1)
>>> time.fromisoformat('04:23:01.000384')
datetime.time(4, 23, 1, 384)
>>> time.fromisoformat('04:23:01,000')
>>> time.fromisoformat('04:23:01,000384')
Copy link
Member

Choose a reason for hiding this comment

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

I think the point of this example is to show the comma-plus-three-digits format, so you want to change the output, not the input.

Though maybe it's better to change this to 04:23:01,678 to make it clear that those are milliseconds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was only a little information for me to interpret the intention of the code example. Given that the returned value of the previous line was the same as this line, I can only guess that this line was meant to show that the comma is strictly equivalent to the dot as the delimiter for fractional seconds. Therefore, I decided to change the returned value of this line.

I think it would be more clear if there were separate examples of showing the equivalence of delimiters and showing the difference between two fractional seconds formats. If this was the case, I would like to keep the current change and add one more code example to demonstrate the 3-digits format for fractional seconds. For example:

>>> time.fromisoformat('04:23:01.384')
datetime.time(4, 23, 1, 384000)

Copy link
Member

Choose a reason for hiding this comment

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

time.fromisoformat('04:23:01.384') was supported in 3.10, but neither time.fromisoformat('04:23:01,000384') nor time.fromisoformat('04:23:01,384') worked. So this new test was for comma, not for three-digits fraction part.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -1786,7 +1786,7 @@ Other constructor:
datetime.time(4, 23, 1)
>>> time.fromisoformat('04:23:01.000384')
datetime.time(4, 23, 1, 384)
>>> time.fromisoformat('04:23:01,000')
>>> time.fromisoformat('04:23:01,000384')
Copy link
Member

Choose a reason for hiding this comment

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

time.fromisoformat('04:23:01.384') was supported in 3.10, but neither time.fromisoformat('04:23:01,000384') nor time.fromisoformat('04:23:01,384') worked. So this new test was for comma, not for three-digits fraction part.

@mikelei8291
Copy link
Contributor Author

Although this PR was opened first, a fix was already merged with the close of issue #112925. :(

@serhiy-storchaka
Copy link
Member

In any way the problem is solved. Thank you for your PR @mikelei8291.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants