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.

Sign up for GitHub

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

Jump to bottom

gh-101100: Fix datetime reference warnings #114661

Merged
merged 12 commits into from
Jan 29, 2024
Merged

gh-101100: Fix datetime reference warnings #114661

merged 12 commits into from
Jan 29, 2024

Conversation

smontanaro
Copy link
Contributor

@smontanaro smontanaro commented Jan 27, 2024 •
edited by github-actions bot
Loading

Fixed dangling references. datetime, date, and time classes have many method names in common (not too surprising). Where it didn't seem to make a difference, I changed the reference to the datetime version of the method.

📚 Documentation preview 📚: https://cpython-previews--114661.org.readthedocs.build/

@bedevere-app bedevere-app bot mentioned this pull request Jan 27, 2024
Open
serhiy-storchaka
serhiy-storchaka reviewed Jan 28, 2024
View reviewed changes
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
@smontanaro
Copy link
Contributor Author

@serhiy-storchaka I applied your suggestions, then got a bit carried away. It seems to me that if :attr:!tm_isdst is appropriate when several references are close together, then self-referential module references should be suppressed in most places. Accordingly, I replaced instances of

:mod:`datetime`

with

:mod:`!datetime`

to suppress the link to the documentation the user is currently viewing. If there's a good reason to retain these links, let me know and I'll undo that part of the change.

serhiy-storchaka
serhiy-storchaka reviewed Jan 28, 2024
View reviewed changes
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
@smontanaro
Copy link
Contributor Author

I've responded to all recent comments.

smontanaro and others added 2 commits January 28, 2024 10:25
@smontanaro
incorrect ref to time class
e46dc4a
@smontanaro @serhiy-storchaka
Apply suggestions from code review
bd1f54d 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka
serhiy-storchaka reviewed Jan 29, 2024
View reviewed changes
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
Doc/library/datetime.rst Outdated Show resolved Hide resolved
@serhiy-storchaka
Copy link
Member

Many references `attr<datetime.attr>` can be shortened to `~.datetime.attr`. Or, if you prefer the long form, they should be written as `~datetime.datetime.attr` or `attr <datetime.datetime.attr>`. Currently they only work by accident and can be broken after adding new global in the datetime module.

smontanaro and others added 2 commits January 29, 2024 07:13
@smontanaro @serhiy-storchaka
Apply suggestions from code review
292dbc1 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@smontanaro
more tilde-prefixed references
fadd32b
serhiy-storchaka
serhiy-storchaka approved these changes Jan 29, 2024
View reviewed changes
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. 😌

@smontanaro
Copy link
Contributor Author

Thanks. Feel free to merge. (I don't have -- or want -- check-in privilege.)

@serhiy-storchaka serhiy-storchaka added docs Documentation in the Doc dir skip news labels Jan 29, 2024
@serhiy-storchaka serhiy-storchaka added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Jan 29, 2024
@serhiy-storchaka serhiy-storchaka merged commit e8b8f5e into python:main Jan 29, 2024
29 checks passed
@miss-islington-app
Copy link

Thanks @smontanaro for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 29, 2024
@smontanaro @serhiy-storchaka @miss-islington
pythongh-101100: Fix datetime reference warnings (pythonGH-114661)
a0eba36 
(cherry picked from commit e8b8f5e)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@miss-islington-app
Copy link

Sorry, @smontanaro and @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e8b8f5e9c2da6a436360ce648061c90bdfcba863 3.11

@bedevere-app
Copy link

bedevere-app bot commented Jan 29, 2024

GH-114716 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Jan 29, 2024
@serhiy-storchaka
Copy link
Member

Thank you @smontanaro. It was a complicated issue.

The documentation can still be refined: there are paragraphs with repeated links. But this is a different issue.

@serhiy-storchaka
Copy link
Member

Please look also at #114717.

serhiy-storchaka added a commit that referenced this pull request Jan 29, 2024
@miss-islington @smontanaro @serhiy-storchaka
[3.12] gh-101100: Fix datetime reference warnings (GH-114661) (GH-114716
783339a 
)

(cherry picked from commit e8b8f5e)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jan 29, 2024
@smontanaro @serhiy-storchaka
[3.11] pythongh-101100: Fix datetime reference warnings (pythonGH-114661
163caee 
)

(cherry picked from commit e8b8f5e)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jan 29, 2024

GH-114718 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Jan 29, 2024
serhiy-storchaka added a commit that referenced this pull request Jan 29, 2024
@serhiy-storchaka @smontanaro
[3.11] gh-101100: Fix datetime reference warnings (GH-114661) (GH-114718
c67f94e 
)

(cherry picked from commit e8b8f5e)

Co-authored-by: Skip Montanaro <skip.montanaro@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@smontanaro @serhiy-storchaka @aisk
pythongh-101100: Fix datetime reference warnings (pythonGH-114661)
3246cec 
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

@abalkin abalkin Awaiting requested review from abalkin abalkin is a code owner

Assignees

@serhiy-storchaka serhiy-storchaka

Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@smontanaro @serhiy-storchaka