◐ Shell
reader mode source ↗
Skip to content

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

Merged
pganssle merged 13 commits into
python:mainfrom
donbarbos:issue-126883
May 19, 2025
Merged

gh-126883: Add check that timezone fields are in range for datetime.fromisoformat#127242
pganssle merged 13 commits into
python:mainfrom
donbarbos:issue-126883

Conversation

@donbarbos

@donbarbos donbarbos commented Nov 25, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor
  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 bedevere-app Bot mentioned this pull request Nov 25, 2024
Closed
@bedevere-app

bedevere-app Bot commented Nov 25, 2024

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the 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

pganssle commented Nov 25, 2024

Copy link
Copy Markdown
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

donbarbos commented Nov 26, 2024

Copy link
Copy Markdown
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 donbarbos mentioned this pull request Nov 30, 2024
Merged
@donbarbos

donbarbos commented Feb 3, 2025

Copy link
Copy Markdown
Contributor Author

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

erlend-aasland
erlend-aasland reviewed Feb 5, 2025
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Can you please remove any unneeded style changes?

donbarbos and others added 3 commits February 5, 2025 08:38
@donbarbos @erlend-aasland
Update Lib/_pydatetime.py …
cc0d774 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@donbarbos
Revert style changes
9adfc6e
@donbarbos
Merge branch 'main' into issue-126883
eac790c
@donbarbos

donbarbos commented Apr 18, 2025

Copy link
Copy Markdown
Contributor Author

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

pganssle
pganssle reviewed May 19, 2025
View reviewed changes

@pganssle pganssle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Some minor nits, but overall looks good!

@donbarbos @pganssle
Apply suggestions from code review …
5580d17 
Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
@donbarbos donbarbos requested a review from pganssle May 19, 2025 16:12
Hide details View details @pganssle pganssle merged commit 71c42b7 into python:main May 19, 2025
46 checks passed
@pganssle

pganssle commented May 19, 2025

Copy link
Copy Markdown
Member

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

@donbarbos

donbarbos commented May 19, 2025

Copy link
Copy Markdown
Contributor Author

thank you everyone for review

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@donbarbos @erlend-aasland
@pganssle @Pranjal095
pythongh-126883: Add check that timezone fields are in range for `dat… …
ef39860 
…etime.fromisoformat` (python#127242)

It was previously possible to specify things like `+00:90:00` which would be equivalent to `+01:30:00`, but is not a valid ISO8601 string.

---------

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@donbarbos @erlend-aasland
@pganssle @taegyunkim
pythongh-126883: Add check that timezone fields are in range for `dat… …
d1a71a4 
…etime.fromisoformat` (python#127242)

It was previously possible to specify things like `+00:90:00` which would be equivalent to `+01:30:00`, but is not a valid ISO8601 string.

---------

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pganssle pganssle pganssle approved these changes

@abalkin abalkin Awaiting requested review from abalkin

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@donbarbos @pganssle @erlend-aasland