Skip to content
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.

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

Remove unused internal _PyImport_GetModuleId() function #107235

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

vstinner
Copy link
Member

No description provided.

@vstinner
Copy link
Member Author

@ericsnowcurrently: Here is a PR to remove _PyImport_GetModuleId().

@ericsnowcurrently
Copy link
Member

FTR, here's Victor's PyPI search: #107213 (comment)

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Assuming no external modules (or embedders) are using this, LGTM.

@vstinner vstinner force-pushed the remove_import_getmoduleid branch from 0caa13c to 9f3f713 Compare July 25, 2023 16:02
@vstinner
Copy link
Member Author

Hum, the CLA Signing job was blocked. So I closed/reopened my PR and also rebased it on the main branch. Let's see if this time, the job starts.

@vstinner
Copy link
Member Author

docs/readthedocs.org:cpython-previews — Read the Docs build failed!

Strange. Python .readthedocs.yaml uses exit 183 if the Python Documentation was not changed and so it's not needed to build the documentation, and this exit code is documented by ReadTheDocs as "skip the build"

@vstinner
Copy link
Member Author

Strange. Python .readthedocs.yaml uses exit 183 if the Python Documentation was not changed and so it's not needed to build the documentation, and this exit code is documented by ReadTheDocs as "skip the build"

I reported the issue to ReadTheDocs bug tracker: readthedocs/readthedocs.org#10567

@vstinner vstinner closed this Jul 25, 2023
@vstinner vstinner reopened this Jul 25, 2023
@vstinner
Copy link
Member Author

Assuming no external modules (or embedders) are using this, LGTM.

I checked again: I found no user in PyPI top 5,000 projects (2023-07-04):

$ ./search_pypi_top.py PYPI-2023-07-04/ '_PyImport_GetModuleId' -q
PYPI-2023-07-04/fastobo-0.12.2.tar.gz: fastobo-0.12.2/crates/pyo3-ffi/src/cpython/import.rs: // skipped _PyImport_GetModuleId
PYPI-2023-07-04/orjson-3.9.1.tar.gz: orjson-3.9.1/include/cargo/pyo3-ffi-0.19.0/src/cpython/import.rs: // skipped _PyImport_GetModuleId

Time: 0:00:39.023925
Found 2 matching lines in 2 projects

The 2 results are only comments to say that PyO3-ffi has no binding for _PyImport_GetModuleId().

@vstinner
Copy link
Member Author

I closed/reopened again the PR to trigger a new ReadTheDocs job. Right now, the job is failing with:

Concurrency limit reached (2), retrying in 5 minutes.

@vstinner vstinner enabled auto-merge (squash) July 25, 2023 17:00
@vstinner vstinner merged commit 188000a into python:main Jul 25, 2023
@vstinner vstinner deleted the remove_import_getmoduleid branch July 25, 2023 17:02
@vstinner
Copy link
Member Author

Merged, thanks for the review @ericsnowcurrently.

jtcave pushed a commit to jtcave/cpython that referenced this pull request Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants