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

deprecated python3 module "imp" used in tools/test.py #29418

Closed
targos opened this issue Sep 3, 2019 · 15 comments
Closed

deprecated python3 module "imp" used in tools/test.py #29418

targos opened this issue Sep 3, 2019 · 15 comments
Labels
python PRs and issues that require attention from people who are familiar with Python.

Comments

@targos
Copy link
Member

targos commented Sep 3, 2019

tools/test.py:32: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses

/cc @cclauss

@targos targos added the python PRs and issues that require attention from people who are familiar with Python. label Sep 3, 2019
@cclauss
Copy link
Contributor

cclauss commented Sep 3, 2019

Closing as duplicate of #29246

@cclauss cclauss closed this as completed Sep 3, 2019
@targos
Copy link
Member Author

targos commented Oct 28, 2019

I'm reopening because #29246 is closed but the warning is still emitted.

@targos targos reopened this Oct 28, 2019
@targos
Copy link
Member Author

targos commented Oct 28, 2019

The imp module is used here to do a dynamic import:

node/tools/test.py

Lines 795 to 805 in 5783ed7

try:
(file, pathname, description) = imp.find_module('testcfg', [ self.path ])
module = imp.load_module('testcfg', file, pathname, description)
self.config = module.GetConfiguration(context, self.path)
if hasattr(self.config, 'additional_flags'):
self.config.additional_flags += context.node_args
else:
self.config.additional_flags = context.node_args
finally:
if file:
file.close()

@cclauss
Copy link
Contributor

cclauss commented Oct 28, 2019

The imp module is fully deprecated in all supported Python 3 versions.

@Hellzed
Copy link
Contributor

Hellzed commented Oct 28, 2019

@targos Try using importlib.find_spec() instead, or importlib.find_loader()

This is from the migration help found here: https://docs.python.org/3/library/imp.html#imp.find_module
And the new module doc: https://docs.python.org/3/library/importlib.html#importlib.util.find_spec

@targos
Copy link
Member Author

targos commented Oct 28, 2019

@Hellzed Thanks. Note that for the time being, we still have to support Python 2.7 and only importlib.import_module seems to exist in that version: https://docs.python.org/2.7/library/importlib.html

@Hellzed
Copy link
Contributor

Hellzed commented Oct 29, 2019

@targos There's no one-size-fits-all solution here. We need to to use sys.version_info to define the exact code that loads modules. Do you need to support Python versions > 2.7 and < 3.4 ?

@cclauss
Copy link
Contributor

cclauss commented Oct 29, 2019

sys.version_info[:2] in ((2, 7), (3, 5), (3, 6), (3, 7), (3, 8))
https://devguide.python.org/#branchstatus

@cclauss cclauss changed the title deprecated python3 module used in tools/test.py deprecated python3 module "imp" used in tools/test.py Nov 1, 2019
@cclauss
Copy link
Contributor

cclauss commented Nov 1, 2019

@asottile
Do you know someone who would be willing to help out Node.js on this one. I tried but failed.

@asottile
Copy link

asottile commented Nov 1, 2019

@asottile
Do you know someone who would be willing to help out Node.js on this one. I tried but failed.

not in particular -- though the solution isn't too difficult. you'd branch on sys.version_info < (3,) and use the importlib machinery in py3+ and imp in python 2.

@cclauss
Copy link
Contributor

cclauss commented Nov 1, 2019

OK... Will try again. Thx.

@Hellzed
Copy link
Contributor

Hellzed commented Nov 1, 2019

I'm on right now, pushing the branch in a moment. Sorry for the delay, busy week.

@Hellzed
Copy link
Contributor

Hellzed commented Nov 1, 2019

Here's the PR: #30208

@cclauss
Copy link
Contributor

cclauss commented Nov 5, 2019

@targos Can we close as fixed in #30208 or are you still seeing this warning?

@targos
Copy link
Member Author

targos commented Nov 5, 2019

it is fixed

@targos targos closed this as completed Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python PRs and issues that require attention from people who are familiar with Python.
Projects
None yet
Development

No branches or pull requests

4 participants