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

TST: coverage overhead mostly goes away in Python >= 3.12 #15975

Closed
neutrinoceros opened this issue Feb 1, 2024 · 11 comments · Fixed by #16466
Closed

TST: coverage overhead mostly goes away in Python >= 3.12 #15975

neutrinoceros opened this issue Feb 1, 2024 · 11 comments · Fixed by #16466
Labels
Enhancement Python 3.12 testing

Comments

@neutrinoceros
Copy link
Contributor

neutrinoceros commented Feb 1, 2024 •
edited

coverage.py is moving to a much more lightweight profiling method, available in Python 3.12 and which current release of coverage.py have an opt-in option for. It'll be the default behaviour sometimes in the future.

I've ran astropy's test suite locally to get a sense of the improvements:

coverage mode run time (s) overhead (%)
None 227 0
7.4.0 (default method) 375 65
7.4.0 (new method) 231 2

This is so much better that I think it would make sense to just have coverage on all the time instead of having dedicated jobs in tox.ini and in CI. However this is not happening before there's at least a nicer interface to use this new method (currently it's controlled by an environment variable and coverage doesn't gracefully fallback in currently-unsupported cases), and even then it'll only apply to Python >= 3.12, so this issue is to serve as a reminder of it when the time comes.

xref: blogpost on this topic by the package author https://nedbatchelder.com/blog/202312/coveragepy_with_sysmonitoring.html
@neutrinoceros
Copy link
Contributor Author

(this isn't a feature request, that label just came from using the feature request issue template, when I should have used free form)

@pllim
Copy link
Member

pllim commented Feb 1, 2024

It is up to you what template you want to use. I updated the labels. Thanks for the heads up!

@pllim
Copy link
Member

pllim commented Feb 1, 2024

I don't think we pull in coverage directly. Is it what pytest-cov uses?

@neutrinoceros
Copy link
Contributor Author

yes it is.

@neutrinoceros neutrinoceros changed the title TST: coverage overhead mostly goes aways in Python >= 3.12 TST: coverage overhead mostly goes away in Python >= 3.12 Feb 4, 2024
@neutrinoceros neutrinoceros mentioned this issue May 16, 2024
Merged
1 task
@pllim
Copy link
Member

pllim commented May 30, 2024

Is this limitation still true? We use plugins.

Some things won’t work with sys.monitoring: plugins and dynamic contexts aren’t yet supported, though eventually they will be. Execution will be faster for line coverage, but not yet for branch coverage. Let me know how it works for you.

@neutrinoceros
Copy link
Contributor Author

What plugins do we use ? I couldn't find any.

@pllim
Copy link
Member

pllim commented May 30, 2024

Does the author mean pytest plugins? We use all the plugins defined here:

astropy/pyproject.toml

Lines 96 to 99 in 42b96fe

"pytest-doctestplus>=0.12",
"pytest-astropy-header>=0.2.1",
"pytest-astropy>=0.10",
"pytest-xdist",

astropy/tox.ini

Line 65 in 42b96fe

image: pytest-mpl>=0.17

@pllim
Copy link
Member

pllim commented May 30, 2024

I am not sure what counts as dynamic context either.

@neutrinoceros
Copy link
Contributor Author

Does the author mean pytest plugins?

No, he means coverage plugins. Pytest and coverage are completely independent tools, we just happen to run the latter through an extension to the former (pytest-cov), which in most cases is actually not necessary, but I digress.

@neutrinoceros
Copy link
Contributor Author

I am not sure what counts as dynamic context either.

I'm assuming he means context managers or something related. I will look it up and ask him for clarification if I'm not confident I get it.

@neutrinoceros
Copy link
Contributor Author

So actually it means something else specifically in coverage: https://coverage.readthedocs.io/en/7.5.3/contexts.html#dynamic-contexts
In short, it's an option that we don't use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement Python 3.12 testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

TST: enable faster coverage in Python>=3.12 neutrinoceros/astropy
2 participants
@pllim @neutrinoceros