Skip to content

bpo-36375: PEP499: implementation and documentation and test updates #12455

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
Closed
wants to merge 8 commits into from
Closed

bpo-36375: PEP499: implementation and documentation and test updates #12455

wants to merge 8 commits into from

Conversation

cameron-simpson
Copy link

@cameron-simpson cameron-simpson commented Mar 20, 2019 •
edited by ncoghlan
Loading

(Superseded, see #12490 instead)

This PR provides an implementation of the -m option semantics change from PEP 499. It also provides some documentation updates and 2 unit tests.

https://bugs.python.org/issue36375

eamanu
eamanu reviewed Mar 20, 2019
View reviewed changes
Misc/NEWS.d/next/Core and Builtins/2019-03-20-00-38-35.bpo-36375.d178R0.rst Outdated
@@ -0,0 +1,8 @@
PEP 499 implemented: the "python -m module_name" command line option now binds the imported module
to both ``sys.modules['__main__'`` and additionally to ``sys.modules['module_name']``.
Copy link
Contributor

Choose a reason for hiding this comment

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

sys.modules['__main__']?

Copy link
Author

Choose a reason for hiding this comment

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

Nice catch, fix applied, thank you! - Cameron

Misc/NEWS.d/next/Core and Builtins/2019-03-20-00-38-35.bpo-36375.d178R0.rst Outdated
which led to hard to debug issues where the object resolved by ``__main__.obj_name``
was distinct from the object resolved by ``module_name.obj_name``.
This led to subtle breakage of code which relied on objtect identity tests,
particularly ``isinstance`` tests and ``try/except`` clauses.
Copy link
Contributor

Choose a reason for hiding this comment

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

I will write isisntance()

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good, applied. Thanks! - Cameron

eamanu
eamanu approved these changes Mar 20, 2019
View reviewed changes
Copy link
Contributor

@eamanu eamanu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please check the travis error

eamanu
eamanu suggested changes Mar 20, 2019
View reviewed changes
Copy link
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

Please review the Travis errors

@cameron-simpson
Copy link
Author

Please review the Travis errors

Hmm. They look related to a problem with hashlib, and not obviously my patch. Would it be sensible for me to rebase the PR on the latest master to trigger a retry? New to this process.

@cameron-simpson
Copy link
Author

Please review the Travis errors

Hmm. They look related to a problem with hashlib, and not obviously my patch. Would it be sensible for me to rebase the PR on the latest master to trigger a retry? New to this process.

Trying it: rebase, see if the travis results change. If not I'll try to reproduce locally.

@cameron-simpson
Copy link
Author

Please review the Travis errors
Hmm. They look related to a problem with hashlib, and not obviously my patch. Would it be sensible for me to rebase the PR on the latest master to trigger a retry? New to this process.
Trying it: rebase, see if the travis results change. If not I'll try to reproduce locally.

Well, no change. And it isn't just hashlib. Investigating further now...

@cameron-simpson
Copy link
Author

cameron-simpson commented Mar 21, 2019 •
edited
Loading

Ok... pdb.Pdb._runmodule depends on __main__ (from "python.exe -m pdb") being a distinct module instance from sys.modules['pdb']. It doesn't actually want that, but it constructs a "bare" __main__ module by rewriting __main__.__dict__ and because that is the pdb module this wipes pdbs brain.

Investigating cleaner ways to do this. Plan A is to del sys.modules['__main__'] and make a new module for that somehow. To keep pdb's other code happy, which may presume __main__ has become the code it is debugging.

@cameron-simpson
Copy link
Author

I'm withdrawing this PR, I've clearly not run the full test suite. I'll bring it back when the tests pass.

@ncoghlan
Copy link
Contributor

@cameron-simpson Ouch. You'll also need to be careful regarding the interactions with the -i switch (which drops into an interactive prompt), as that always runs the interactive session in the original __main__ (this is one of the reasons why pdb works the way it does)

@cameron-simpson cameron-simpson mannequin mentioned this pull request May 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eamanu eamanu eamanu requested changes

Assignees
No one assigned
Labels
awaiting core review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cameron-simpson @ncoghlan @eamanu @the-knights-who-say-ni @bedevere-bot