GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-30535: Explicitly note that sys.meta_path is not empty by Windsooon · Pull Request #13300 · python/cpython

Conversation

@Windsooon

@Windsooon Windsooon commented May 14, 2019 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Explicitly note that sys.meta_path is not empty

https://bugs.python.org/issue30535

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 14, 2019
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels May 14, 2019
@csabella
csabella requested a review from brettcannon May 14, 2019 09:59
jdemeyer
jdemeyer requested changes May 16, 2019
View reviewed changes
Comment thread Doc/library/sys.rst Outdated

@jdemeyer jdemeyer May 16, 2019

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Windsooon Windsooon May 16, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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...)."

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels May 16, 2019
@Windsooon
bpo-30535: Explicitly note that sys.meta_path is not empty
aa843d1
@Windsooon
Windsooon force-pushed the bpo-30535 branch from 236f433 to aa843d1 Compare May 17, 2019 01:40
jdemeyer
jdemeyer approved these changes May 18, 2019
View reviewed changes
brettcannon
brettcannon requested changes May 27, 2019
View reviewed changes

@brettcannon brettcannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/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

@brettcannon brettcannon May 27, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Windsooon Windsooon May 28, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@brettcannon brettcannon Nov 22, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot removed the awaiting core review label May 27, 2019
@bedevere-bot

bedevere-bot commented May 27, 2019

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting changes label May 27, 2019
@brettcannon brettcannon added the skip news label May 27, 2019
@csabella
csabella requested a review from brettcannon November 20, 2019 01:47
brettcannon
brettcannon requested changes Nov 22, 2019
View reviewed changes

@brettcannon brettcannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@csabella

csabella commented Jan 26, 2020

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood

AlexWaygood commented Apr 19, 2022

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! 🙂
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood AlexWaygood closed this Apr 19, 2022
@iritkatriel iritkatriel reopened this Jun 21, 2022
@iritkatriel

iritkatriel commented Jun 21, 2022

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@iritkatriel iritkatriel closed this Jun 21, 2022
@iritkatriel iritkatriel mentioned this pull request Jun 21, 2022
bpo-30535: [doc] state that sys.meta_path is not empty by default #94098
Merged
@masklinn masklinn mannequin mentioned this pull request Jun 21, 2022
[doc] Explicitly note that meta_path is not empty #74720
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon brettcannon requested changes
@jdemeyer jdemeyer jdemeyer approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@Windsooon @bedevere-bot @csabella @AlexWaygood @iritkatriel @brettcannon @jdemeyer @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.