Skip to content

importlib doc: Fix required Python version of an example#10030

Closed
e-kwsm wants to merge 1 commit into
python:masterfrom
e-kwsm:fix-docs
Closed

importlib doc: Fix required Python version of an example#10030
e-kwsm wants to merge 1 commit into
python:masterfrom
e-kwsm:fix-docs

Conversation

@e-kwsm

@e-kwsm e-kwsm commented Oct 21, 2018

Copy link
Copy Markdown
Contributor

No description provided.

§31.5.6.3. Importing a source file directly
`module_from_spec` is new in 3.5
[ci skip]
@bedevere-bot bedevere-bot added the docs Documentation in the Doc dir label Oct 21, 2018
@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@vstinner

Copy link
Copy Markdown
Member

I closed/reopened the PR to schedule a new AppVeyor job.

@vstinner vstinner changed the title [Docs] Fix required version of an example of importlib importlib doc: Fix required Python version of an example Oct 24, 2018
@vstinner

Copy link
Copy Markdown
Member

Copy of the original description:

31.5.6.3. Importing a source file directly
To import a Python source file directly, use the following recipe (Python 3.4 and newer only):

import importlib.util
import sys

# For illustrative purposes.
import tokenize
file_path = tokenize.__file__
module_name = tokenize.__name__

spec = importlib.util.spec_from_file_location(module_name, file_path)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
# Optional; only necessary if you want to be able to import the module
# by name later.
sys.modules[module_name] = module

The above example requires at least 3.5 because of module_from_spec.

The example code in 31.5.6.5. Approximating importlib.import_module() also uses module_from_spec, but I'm not sure it is "the importlib usage", so didn't modify.

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

LGTM.

@vstinner

Copy link
Copy Markdown
Member

This change will be merged once the CI pass, please don't change the title nor the description.

@e-kwsm

e-kwsm commented Oct 25, 2018

Copy link
Copy Markdown
Contributor Author

This change will be merged once the CI pass,

Do I have to remove [ci skip] from the commit message?

@vstinner vstinner closed this Oct 25, 2018
@vstinner vstinner reopened this Oct 25, 2018
@vstinner

Copy link
Copy Markdown
Member

Sadly, the AppVeyor CI job never started :-( I closed/reopened the PR to schedule new CI jobs.

@vstinner

Copy link
Copy Markdown
Member

Do I have to remove [ci skip] from the commit message?

"automerge" uses the PR title and description.

@vstinner

Copy link
Copy Markdown
Member

@e-kwsm: I am sorry, but something is broken with this PR. CI jobs never start. I close it. Please create a new PR. Since it's a single character, it should be easy :-)

Then add the comment "cc @vstinner" to make sure that I don't miss your new PR ;-)

@e-kwsm

e-kwsm commented Oct 25, 2018

Copy link
Copy Markdown
Contributor Author

[ci skip] prevents AppVeyor CI.

I thought this is very "trivial" and unnecessary to run CI 😰.

I removed [ci skip] and git push -f but the commit message in this PR does not change, probably because this is closed.

@e-kwsm

e-kwsm commented Oct 25, 2018

Copy link
Copy Markdown
Contributor Author

@vstinner I'm sorry but would you reopen?

@vstinner

Copy link
Copy Markdown
Member

Please just create a new one, it will be simpler for me.

@e-kwsm

e-kwsm commented Oct 28, 2018

Copy link
Copy Markdown
Contributor Author

@vstinner I created #10118. Would you check if the CI works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants