FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-125633: Add function `ispackage` to stdlib `inspect` by Xiaokang2022 · Pull Request #125634 · python/cpython · GitHub

/ cpython Public

gh-125633: Add function ispackage to stdlib inspect - #125634

Merged
ncoghlan merged 14 commits into
python:mainfrom
Xiaokang2022:add-ispackage
Oct 27, 2024
ConversationCommits (14)ChecksFiles changed
Merged

gh-125633: Add function ispackage to stdlib inspect#125634
ncoghlan merged 14 commits into
python:mainfrom
Xiaokang2022:add-ispackage

Conversation

Xiaokang2022 commented Oct 17, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

bedevere-app Bot mentioned this pull request Oct 17, 2024
Closed
nineteendo reviewed Oct 17, 2024
View reviewed changes

nineteendo left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you add tests and fix the sphinx markup?

Comment thread Misc/NEWS.d/next/Library/2024-10-17-04-52-00.gh-issue-125633.lMck06.rst Outdated

Wulian233 commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

Please add a unittest and document.

Also add to 3.14 what's new(recommend)

tomasr8 commented Oct 17, 2024

Copy link
Copy Markdown
Member

One of the tests is currently failing because you also need to add the new test folder to the Makefile:

cpython/Makefile.pre.in

Line 2500 in 7b04496

test/test_inspect \

should be:

  test/test_inspect \
+ test/test_inspect/inspect_simple_pkg \

Xiaokang2022 changed the title gh-125633: Adding function ispackage to stdlib inspect gh-125633: Add function ispackage to stdlib inspect Oct 17, 2024

Wulian233 commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

LGTM

vstinner approved these changes Oct 17, 2024
View reviewed changes

vstinner 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

Comment thread Doc/library/inspect.rst
vstinner reviewed Oct 17, 2024
View reviewed changes
Comment thread Doc/library/inspect.rst Outdated
Comment thread Doc/whatsnew/3.14.rst Outdated
Eclips4 reviewed Oct 17, 2024
View reviewed changes
Comment thread Lib/inspect.py Outdated
Eclips4 approved these changes Oct 17, 2024
View reviewed changes

Eclips4 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks!

vstinner commented Oct 17, 2024

Copy link
Copy Markdown
Member

@brettcannon @ncoghlan: Is it the correct way to check if a module a package, ismodule(object) and hasattr(object, "__path__")?

sobolevn reviewed Oct 19, 2024
View reviewed changes

sobolevn 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks! Several comments :)

Comment thread Lib/test/test_inspect/test_inspect.py
Comment thread Lib/test/test_inspect/test_inspect.py Outdated
Comment thread Makefile.pre.in Outdated
sobolevn reviewed Oct 19, 2024
View reviewed changes
Comment thread Lib/test/test_inspect/test_inspect.py Outdated
erlend-aasland removed their request for review October 21, 2024 10:08

ncoghlan commented Oct 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

I'm not sure __path__ is mandatory anymore, as I believe it's just a way to override __spec__. I'll double check the importlib code.

Edit: after checking the importlib code, I'm happy simply looking for __path__ is still the right check for runtime introspection. importlib itself handles merging __path__, __spec__.is_package and __spec__.submodule_search_locations into a single final __path__ list when it creates the import package module instances.

This also keeps things consistent with the glossary's definition of a package as a module with a __path__ attribute.

ncoghlan merged commit dad3453 into python:main Oct 27, 2024

ncoghlan commented Oct 27, 2024

Copy link
Copy Markdown
Contributor

Thanks for the PR @Xiaokang2022, and thanks for the prior reviews all!

ncoghlan reviewed Oct 27, 2024
View reviewed changes
Comment thread Lib/test/test_inspect/test_inspect.py
self.istest(inspect.ispackage, 'importlib')
self.assertFalse(inspect.ispackage(inspect))
self.assertFalse(inspect.ispackage(mod))
self.assertFalse(inspect.ispackage(':)'))

ncoghlan Oct 27, 2024

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This last test genuinely made me laugh :)

Xiaokang2022 deleted the add-ispackage branch January 6, 2025 04:40
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
pythongh-125633: Add function ispackage to stdlib inspect (python…
6ed9518 
…#125634)

---------

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner approved these changes
ncoghlan ncoghlan approved these changes
sobolevn sobolevn left review comments
Eclips4 Eclips4 approved these changes
+2 more reviewers
nineteendo nineteendo left review comments
Wulian233 Wulian233 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL