Skip to content

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

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
Merged
merged 14 commits into from
Oct 27, 2024
Merged

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

merged 14 commits into from
Oct 27, 2024

Conversation

Xiaokang2022
Copy link
Contributor

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

@bedevere-app bedevere-app bot mentioned this pull request Oct 17, 2024
Closed
nineteendo
nineteendo reviewed Oct 17, 2024
View reviewed changes
Copy link
Contributor

@nineteendo nineteendo left a comment

Choose a reason for hiding this comment

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

Can you add tests and fix the sphinx markup?

@Wulian233
Copy link
Contributor

Please add a unittest and document.

Also add to 3.14 what's new(recommend)

@tomasr8
Copy link
Member

tomasr8 commented Oct 17, 2024

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 Xiaokang2022 changed the title gh-125633: Adding function ispackage to stdlib inspect gh-125633: Add function ispackage to stdlib inspect Oct 17, 2024
@Wulian233
Copy link
Contributor

LGTM

vstinner
vstinner approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

vstinner
vstinner reviewed Oct 17, 2024
View reviewed changes
Eclips4
Eclips4 reviewed Oct 17, 2024
View reviewed changes
Eclips4
Eclips4 approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

Thanks!

@vstinner
Copy link
Member

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

sobolevn
sobolevn reviewed Oct 19, 2024
View reviewed changes
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Several comments :)

sobolevn
sobolevn reviewed Oct 19, 2024
View reviewed changes
@erlend-aasland erlend-aasland removed their request for review October 21, 2024 10:08
@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 27, 2024 •
edited
Loading

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 ncoghlan merged commit dad3453 into python:main Oct 27, 2024
34 checks passed
@ncoghlan
Copy link
Contributor

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

ncoghlan
ncoghlan reviewed Oct 27, 2024
View reviewed changes
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(':)'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This last test genuinely made me laugh :)

@Xiaokang2022 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
@Xiaokang2022 @blurb-it @ebonnal
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

@ncoghlan ncoghlan ncoghlan approved these changes

@sobolevn sobolevn sobolevn left review comments

@nineteendo nineteendo nineteendo left review comments

@Wulian233 Wulian233 Wulian233 approved these changes

@Eclips4 Eclips4 Eclips4 approved these changes

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
@Xiaokang2022 @Wulian233 @tomasr8 @vstinner @ncoghlan @sobolevn @nineteendo @Eclips4