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

Add a document describing when to use ciso8601 #128

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

movermeyer
Copy link
Collaborator

@movermeyer movermeyer commented Nov 17, 2022

What are you trying to accomplish?

cPython 3.11's datetime.fromisoformat method is finally feature-full enough to be worthy of comparison with ciso8601, and indeed at present outperforms ciso8601 for timestamps without timezones (Until #130 gets merged).

Finally, many Python users have no need for ciso8601 at all! 🎉

We're going to need some writing to let people understand the cases in which they might not need ciso8601.

What approach did you choose and why?

I wrote a new document that outlines the relevant considerations when choosing a ISO 8601 timestamp parser.

I used GitHub's support for Mermaid diagrams to produce a flow chart. That was really pleasant; I found Mermaid to be delightful and intuitive.

The diagram will become much simpler if I:

As there would be fewer decision points, and fewer reasons to not use ciso8601.

What should reviewers focus on?

Copy editing.

The impact of these changes

Hopefully, users will become aware that they might not need ciso8601 at all, and can use the built-in datetime.fromisoformat method in many cases.

@movermeyer movermeyer force-pushed the movermeyer/why_ciso8601 branch 7 times, most recently from ea845f7 to d7bffa3 Compare November 18, 2022 01:38
@movermeyer movermeyer marked this pull request as ready for review November 18, 2022 01:46
@movermeyer movermeyer force-pushed the movermeyer/why_ciso8601 branch 3 times, most recently from 0ea6137 to 7877784 Compare November 19, 2022 03:54
@AlecRosenbaum AlecRosenbaum removed the request for review from thomasst November 22, 2022 18:10
@movermeyer movermeyer merged commit 5316a93 into master Nov 22, 2022
@movermeyer movermeyer deleted the movermeyer/why_ciso8601 branch December 21, 2022 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants