Skip to content

bpo-30535: Explicitly note that sys.meta_path is not empty - #13300

Closed
Windsooon wants to merge 1 commit into
python:mainfrom
Windsooon:bpo-30535
Closed

bpo-30535: Explicitly note that sys.meta_path is not empty#13300
Windsooon wants to merge 1 commit into
python:mainfrom
Windsooon:bpo-30535

Conversation

@Windsooon

@WindsooonWindsooon commented May 14, 2019

Copy link
Copy Markdown
Contributor

Explicitly note that sys.meta_path is not empty

https://bugs.python.org/issue30535

Comment threadDoc/library/sys.rst Outdated

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.

This is not grammatically correct: a sentence should start with a capital letter and contain a verb.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thank you, how about
"By default, it holds entries to handle the standard kinds of modules (.py files, extension modules...)."

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

Technically there aren't different kinds of modules, just different ways of storing code. I have suggested a tweak to the wording.

Comment threadDoc/library/sys.rst
:meth:`~importlib.abc.MetaPathFinder.find_spec` methods called to see if one
of the objects can find the module to be imported. The
of the objects can find the module to be imported. By default, it holds entries
to handle the standard kinds of modules (.py files, extension modules...). The

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.

Suggested change
to handle the standard kinds of modules (.py files, extension modules...). The
that implement Python's default import semantics. The

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@brettcannon Do you think we should add examples after that. Like:

that implement Python's default import semantics. (BuiltinImporter, FrozenImporter, and PathFinder)

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.

No, as that's too easy of a thing to forget to update. Plus there's no guarantee that every interpreter will set up sys.meta_path the same.

@bedevere-bot

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.

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

I don't think the docs should get too specific as interpreters can choose what to put into sys.meta_path.

@csabella

Copy link
Copy Markdown
Contributor

@Windsooon, please take a look at the review comments. Thanks!

@AlexWaygood

Copy link
Copy Markdown
Member

Changes were requested by a core dev over two years ago, but have not been made. I am therefore closing this PR.

@Windsooon, if you're still interested in working on this, feel free to open a new PR. Alternatively, ping me, and I'll happily reopen this PR. Thanks! 🙂

@iritkatriel

Copy link
Copy Markdown
Member

I tried to updated this PR with Brett's comment but I don't think I can. I will create a new PR.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changesdocsDocumentation in the Doc dirskip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@Windsooon@bedevere-bot@csabella@AlexWaygood@iritkatriel@brettcannon@jdemeyer@the-knights-who-say-ni