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.

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

Replace importlib_metadata with importlib.metadata on Python 3.8+ #223

Merged
merged 3 commits into from Jul 3, 2019
Merged

Replace importlib_metadata with importlib.metadata on Python 3.8+ #223

merged 3 commits into from Jul 3, 2019

Conversation

hroncok
Copy link
Member

@hroncok hroncok commented Jul 2, 2019

Fixes #222

Got pytest: error: unrecognized arguments: -ra with tox on 3.8 :(

@nicoddemus
Copy link
Member

Got pytest: error: unrecognized arguments: -ra with tox on 3.8 :(

Yes this is a known bug in 3.8, which was fixed by pytest-dev/pytest#5523 but not yet released. Could you please change -ra to -r a?

nicoddemus
nicoddemus requested changes Jul 2, 2019
View reviewed changes
testing/test_pluginmanager.py Outdated Show resolved Hide resolved
@nicoddemus
Copy link
Member

Got pytest: error: unrecognized arguments: -ra with tox on 3.8 :(

Yes this is a known bug in 3.8, which was fixed by pytest-dev/pytest#5523 but not yet released. Could you please change -ra to -r a?

@nicoddemus
Copy link
Member

Thanks @hroncok! 👍

@hroncok
Copy link
Member Author

hroncok commented Jul 2, 2019 •
edited

OK. The tox/travis configuration also probably needs some teaks:

  • there is no 3.8 on travis (I've missed it, it is there)
  • there is no 3.7 nor 3.8 in tox
  • py27-pytest{master,features} no longer works

@nicoddemus
Copy link
Member

Agreed, we should update the configuration after pytest 5.0 has been released.

You want to do the tweaks yourself, or would you like me to do it?

@hroncok
Copy link
Member Author

hroncok commented Jul 2, 2019

Do you mind me pushing it here?

@blueyed
Copy link
Contributor

blueyed commented Jul 2, 2019 •
edited

there is no 3.8 on travis

It is?! - but should be removed from allowed failures maybe?

Apart from that makes sense to add it here.

@hroncok
Copy link
Member Author

hroncok commented Jul 2, 2019

there is no 3.8 on travis

yep, crossed that out right after I've posted it. sorry for the confusion

@nicoddemus
Copy link
Member

Do you mind me pushing it here?

Not at all, but if possible make that in a separate commit. 👍

@hroncok
Copy link
Member Author

hroncok commented Jul 2, 2019

Not sure if appveyor has 3.8, we'll see.

nicoddemus
nicoddemus approved these changes Jul 2, 2019
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @hroncok!

@hroncok
Copy link
Member Author

hroncok commented Jul 2, 2019

appveyor: ERROR: InterpreterNotFound: python3.8

@codecov-io
Copy link

codecov-io commented Jul 3, 2019 •
edited

Codecov Report

Merging #223 into master will increase coverage by 0.74%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #223      +/-   ##
==========================================
+ Coverage   93.14%   93.88%   +0.74%     
==========================================
  Files          14        9       -5     
  Lines        1678     1129     -549     
  Branches      116       21      -95     
==========================================
- Hits         1563     1060     -503     
+ Misses         98       66      -32     
+ Partials       17        3      -14
Impacted Files Coverage Δ
testing/test_pluginmanager.py 96.02% <100%> (+0.02%) ⬆️
src/pluggy/hooks.py
src/pluggy/_tracing.py
src/pluggy/__init__.py
src/pluggy/callers.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57b2a6d...1bf30d6. Read the comment docs.

@nicoddemus nicoddemus merged commit 8aa5592 into pytest-dev:master Jul 3, 2019
@nicoddemus
Copy link
Member

Thanks @hroncok!

Do you need a new release with this change?

@hroncok hroncok deleted the importlib_metadata branch July 3, 2019 10:54
@hroncok
Copy link
Member Author

hroncok commented Jul 3, 2019

Thanks for help. No if it is not needed for pytest-dev/pytest#5537

@hroncok hroncok mentioned this pull request Jul 8, 2019
Closed
blueyed
blueyed reviewed Aug 20, 2019
View reviewed changes
testing/test_pluginmanager.py
from importlib import metadata
else:
import importlib_metadata as metadata

Copy link
Contributor

@blueyed blueyed Aug 20, 2019 •
edited

Choose a reason for hiding this comment

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

Just wondered if this could/should import/use it from src/pluggy/manager.py?
=> #230

@hroncok hroncok mentioned this pull request Apr 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blueyed blueyed blueyed left review comments

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

Use standard library importlib.metadata on Python 3.8
4 participants
@hroncok @nicoddemus @blueyed @codecov-io