Skip to content

gh-126883: Add check that timezone fields are in range for datetime.fromisoformat #127242

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

Merged
merged 13 commits into from
May 19, 2025

Conversation

donBarbos
Copy link
Contributor

@donBarbos donBarbos commented Nov 25, 2024

  1. Update C implementation
  2. Update Python implementation
  3. Add more use cases to tests

Also error output for the time.fromisoformat function was fixed as in datetime.fromisoformat in Python implementation

@bedevere-app
Copy link

bedevere-app bot commented Nov 25, 2024

Most changes to Python blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@pganssle
Copy link
Member

I haven't had a chance for a through review yet, but I'm in favor of the general approach and a casual glance looks good.

Can you add a news blurb (credit yourself appropriately unless you don't want that for some reason), and if you aren't already in ACKS please add yourself there (again unless you don't want to be acknowledged for some reason).

@donBarbos
Copy link
Contributor Author

@pganssle done 👍
I think it's now ready for review and merging.
And I'll soon send a PR to solve the rest of the problems described here #127260 and I would be grateful if you answered in the issue.

@donBarbos
Copy link
Contributor Author

@erlend-aasland maybe you have free time to review and merge it

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Can you please remove any unneeded style changes?

donBarbos and others added 3 commits February 5, 2025 08:38
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@donBarbos
Copy link
Contributor Author

cc @erlend-aasland
sorry for another ping but this was one of my first PRs and it's a matter of principle :)

pganssle
Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

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

Some minor nits, but overall looks good!

Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
@donBarbos donBarbos requested a review from pganssle May 19, 2025 16:12
@pganssle pganssle merged commit 71c42b7 into python:main May 19, 2025
46 checks passed
@pganssle
Copy link
Member

Thanks @donBarbos, great work on this and sorry for the long delay.

@donBarbos
Copy link
Contributor Author

thank you everyone for review

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.

3 participants

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant