Skip to content

gh-103066: Add links and help in site.py constants#103777

Merged
hugovk merged 8 commits into
python:mainfrom
davidcaron:docs-site-constants
Sep 6, 2024
Merged

gh-103066: Add links and help in site.py constants#103777
hugovk merged 8 commits into
python:mainfrom
davidcaron:docs-site-constants

Conversation

@davidcaron

@davidcaron davidcaron commented Apr 24, 2023

Copy link
Copy Markdown
Contributor

Fixes issue: #103066

The issue mentioned that the site.py documentation didn't have a list of objects it adds to builtins.

I found this list exists somewhere else in the documentation (built-in constants), so I figured linking to it was simpler.

Also the help special constant wasn't mentioned in this list of site-added constants, so I added it with a link to the built-in functions section where it's documented more extensively.

@ghost

ghost commented Apr 24, 2023

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

@arhadthedev

Copy link
Copy Markdown
Member

@terryjreedy as a participant of the parent issue.

@serhiy-storchaka serhiy-storchaka 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.

help() is already documented in functions.rst. Instead of documenting it again as a "constant", it may be better either to mention it explicitly when referring to the added builtins:

and add :func:`help` and :ref:`few other builtins <site-consts>`

or add a reference to it in the description of the added constants:

adds the :func:`help` function and several constants to the built-in namespace.

@terryjreedy terryjreedy 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.

My suggestions in response to Serhiy's comments.

Comment thread Doc/library/constants.rst
:exc:`SyntaxError`), so they can be considered "true" constants.


.. _site-consts:

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.

The objects above are all non-callable singleton or doubleton instances of their class. (debug is one of False or True). All of the below are callable singleton or doubleton instances of custom classes in _sitebuiltins. I would prefer that 'constants' be replaced by 'objects'/'callables'/'functions'/???, but this is secondary to the issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree with your observation, they are not exactly constants. As the title of this section is "Built-in Constants", I won't make any changes, I feel it's a bit out of scope.

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.

I would take the opportunity to improve this to say that the site module adds several globals to the built-in namespace

Comment thread Doc/library/constants.rst Outdated
Comment thread Doc/library/constants.rst Outdated
Comment thread Doc/library/site.rst Outdated
Comment thread Doc/library/site.rst Outdated
@bedevere-app

bedevere-app Bot commented Feb 3, 2024

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@blaisep

blaisep commented May 20, 2024

Copy link
Copy Markdown
Contributor

Hi @davidcaron , thank you for the contribution.
Are you still interested in applying the suggestions from @terryjreedy and @serhiy-storchaka ?

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

davidcaron and others added 2 commits May 26, 2024 16:39
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

@davidcaron davidcaron left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for taking the time to review this! I agree with your comments, I believe they are spot on. Let me know if there is anything else.

Comment thread Doc/library/constants.rst
:exc:`SyntaxError`), so they can be considered "true" constants.


.. _site-consts:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree with your observation, they are not exactly constants. As the title of this section is "Built-in Constants", I won't make any changes, I feel it's a bit out of scope.

@davidcaron

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented May 26, 2024

Copy link
Copy Markdown

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

@bedevere-app
bedevere-app Bot requested a review from terryjreedy May 26, 2024 20:47
@terryjreedy

Copy link
Copy Markdown
Member

@serhiy-storchaka The only docs failure reason I could find in the raw log, check-warnings.py: error: unrecognized arguments: --fail-if-new-news-nit seems not related to PR. Otherwise, merge if you think OK.

@hugovk

hugovk commented Sep 5, 2024

Copy link
Copy Markdown
Member

Does this still apply to the new REPL?

https://docs.python.org/3.13/whatsnew/3.13.html#a-better-interactive-interpreter

@hugovk hugovk added the topic-repl Related to the interactive shell label Sep 5, 2024
@merwok

merwok commented Sep 5, 2024

Copy link
Copy Markdown
Member

I don’t see any difference for help in the new REPL!

@hugovk

hugovk commented Sep 6, 2024

Copy link
Copy Markdown
Member

There's a difference with -S:

❯ python3.12 -S
Python 3.12.5 (v3.12.5:ff3bc82f7c9, Aug  7 2024, 05:32:06) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
>>> help(dir)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'help' is not defined
>>> help
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'help' is not defined
>>> ^D
❯ python3.13 -S
Python 3.13.0rc1 (v3.13.0rc1:e4a3e786a5e, Jul 31 2024, 19:49:53) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
>>> help(dir)
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    help(dir)
    ^^^^
NameError: name 'help' is not defined
>>> help
>>> Welcome to Python 3.13's help utility! If this is your first time using
Python, you should definitely check out the tutorial at
https://docs.python.org/3.13/tutorial/.

Enter the name of any module, keyword, or topic to get help on writing
Python programs and using Python modules.  To get a list of available
modules, keywords, symbols, or topics, enter "modules", "keywords",
"symbols", or "topics".

Each module also comes with a one-line summary of what it does; to list
the modules whose name or summary contain a given string such as "spam",
enter "modules spam".

To quit this help utility and return to the interpreter,
enter "q", "quit" or "exit".

help>

So this text may be technically correct, in that the help() function is blocked:

Importing this module normally appends site-specific paths to the module search path and adds callables, including help() to the built-in namespace. However, Python startup option -S blocks this and this module can be safely imported with no automatic modifications to the module search path or additions to the builtins.

Maybe it's confusing that the help keyword still has effect.

Is this a bug in the new REPL?

@ambv

ambv commented Sep 6, 2024

Copy link
Copy Markdown
Contributor

This is not a bug. When -S prevents site.py from running, the visible effect is fewer imported modules and fewer entries in globals. That's the intended goal.

While that means that the old-style help() and exit() callables are gone in this scenario, making the functionality itself unavailable was not the goal.

Therefore, it's perfectly fine that the new-style commands (that do not require entries in globals) continue to work.

@hugovk hugovk 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.

Thanks all, then let's merge this!

@hugovk
hugovk merged commit 67957ea into python:main Sep 6, 2024
@hugovk hugovk added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Sep 6, 2024
@miss-islington-app

Copy link
Copy Markdown

Thanks @davidcaron for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Thanks @davidcaron for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 6, 2024
…103777)

(cherry picked from commit 67957ea)

Co-authored-by: David Caron <dcaron05@gmail.com>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 6, 2024
…103777)

(cherry picked from commit 67957ea)

Co-authored-by: David Caron <dcaron05@gmail.com>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@bedevere-app

bedevere-app Bot commented Sep 6, 2024

Copy link
Copy Markdown

GH-123762 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Sep 6, 2024
@bedevere-app

bedevere-app Bot commented Sep 6, 2024

Copy link
Copy Markdown

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

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Sep 6, 2024
hugovk pushed a commit that referenced this pull request Sep 6, 2024
… (#123762)

Co-authored-by: David Caron <dcaron05@gmail.com>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
hugovk pushed a commit that referenced this pull request Sep 6, 2024
… (#123763)

Co-authored-by: David Caron <dcaron05@gmail.com>
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news topic-repl Related to the interactive shell

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants