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 — gh-125633: Add function `ispackage` to stdlib `inspect` by Xiaokang2022 · Pull Request #125634 · python/cpython

Conversation

@Xiaokang2022

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

Uh oh!

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

Copy link
Copy Markdown
Contributor
  • Issue: Add function ispackage to stdlib inspect #125633

Sorry, something went wrong.

Uh oh!

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

Wulian233 reacted with rocket emoji
All reactions
  • 🚀 1 reaction
@Xiaokang2022
Add function ispackage to stdlib inspect
12cb666
@bedevere-app bedevere-app Bot added the awaiting review label Oct 17, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Oct 17, 2024
Add function ispackage to stdlib inspect #125633
Closed
@blurb-it
📜🤖 Added by blurb_it.
2105a10
nineteendo
nineteendo reviewed Oct 17, 2024
View reviewed changes

@nineteendo nineteendo left a comment

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.

Can you add tests and fix the sphinx markup?

Sorry, something went wrong.

Uh oh!

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

Xiaokang2022 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Misc/NEWS.d/next/Library/2024-10-17-04-52-00.gh-issue-125633.lMck06.rst Resolved

Uh oh!

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

@Xiaokang2022
Fixed the sphinx markup
efa661d
@Wulian233

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)

Xiaokang2022 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

Xiaokang2022 added 3 commits October 17, 2024 18:35
@Xiaokang2022
Added tests
40d5a49
@Xiaokang2022
Added document
658ef94
@Xiaokang2022
Added to 3.14 what's new
0943c62
@tomasr8

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 and Wulian233 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@Xiaokang2022
Added the new test folder to the Makefile
a0b64f7
@Xiaokang2022
Xiaokang2022 requested a review from erlend-aasland as a code owner October 17, 2024 11:30
@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

Wulian233 commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

LGTM

All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner
vstinner approved these changes Oct 17, 2024
View reviewed changes

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

LGTM.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Doc/library/inspect.rst Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Oct 17, 2024
@Xiaokang2022
Added the missing markup
4c183ee
Eclips4
Eclips4 reviewed Oct 17, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-17-04-52-00.gh-issue-125633.lMck06.rst Resolved

Uh oh!

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

@Xiaokang2022
Changed news
a2eb627
vstinner
vstinner reviewed Oct 17, 2024
View reviewed changes
Comment thread Doc/library/inspect.rst Resolved

Uh oh!

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

Comment thread Doc/whatsnew/3.14.rst Resolved

Uh oh!

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

Xiaokang2022 added 2 commits October 18, 2024 00:18
@Xiaokang2022
Changed 3.14 what's new
c256e01
@Xiaokang2022
Changed doc
20f497f
Eclips4
Eclips4 reviewed Oct 17, 2024
View reviewed changes
Comment thread Lib/inspect.py Resolved

Uh oh!

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

@Xiaokang2022
Improved the code
ef7eebd
Eclips4
Eclips4 approved these changes Oct 17, 2024
View reviewed changes

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

Thanks!

Sorry, something went wrong.

Uh oh!

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

All reactions
@vstinner

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__")?

All reactions

Sorry, something went wrong.

Uh oh!

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

Wulian233
Wulian233 approved these changes Oct 19, 2024
View reviewed changes
sobolevn
sobolevn reviewed Oct 19, 2024
View reviewed changes

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

Thanks! Several comments :)

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/test/test_inspect/test_inspect.py Resolved

Uh oh!

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

Comment thread Lib/test/test_inspect/test_inspect.py Resolved

Uh oh!

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

Comment thread Makefile.pre.in Resolved

Uh oh!

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

@Xiaokang2022
Improved tests
adfc70b
sobolevn
sobolevn reviewed Oct 19, 2024
View reviewed changes
Comment thread Lib/test/test_inspect/test_inspect.py Resolved

Uh oh!

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

@Xiaokang2022
Improved tests
95ac2c6
@erlend-aasland
erlend-aasland removed their request for review October 21, 2024 10:08
@ncoghlan

ncoghlan commented Oct 27, 2024 •
edited
Loading

Uh oh!

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

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.

Xiaokang2022 and Eclips4 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

ncoghlan
ncoghlan approved these changes Oct 27, 2024
View reviewed changes
@ncoghlan
ncoghlan merged commit dad3453 into python:main Oct 27, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 27, 2024
@ncoghlan

ncoghlan commented Oct 27, 2024

Copy link
Copy Markdown
Contributor

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

vstinner reacted with thumbs up emoji Xiaokang2022 reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

ncoghlan
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 ncoghlan Oct 27, 2024

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 last test genuinely made me laugh :)

Sorry, something went wrong.

Uh oh!

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

Xiaokang2022, Wulian233, and Eclips4 reacted with laugh emoji
All reactions
  • 😄 3 reactions
@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
@Eclips4 Eclips4 Eclips4 approved these changes
+2 more reviewers
@nineteendo nineteendo nineteendo left review comments
@Wulian233 Wulian233 Wulian233 approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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

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