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

ENH: Improve warning stacklevel #12014

Merged
merged 5 commits into from
Feb 23, 2024
Merged

ENH: Improve warning stacklevel #12014

merged 5 commits into from
Feb 23, 2024

Conversation

larsoner
Copy link
Contributor

@larsoner larsoner commented Feb 20, 2024

  • Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.
  • Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.
  • Add yourself to AUTHORS in alphabetical order.

This is really proposal-by-PR (because the PR is pretty trivial). In MNE-Python at least it takes this warning on 8.0.1:

================================================================================= warnings summary =================================================================================
../pytest/src/_pytest/mark/structures.py:357
  /home/larsoner/python/pytest/src/_pytest/mark/structures.py:357: PytestRemovedIn9Warning: Marks applied to fixtures have no effect
  See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function
    store_mark(func, self.mark)

and puts it on the correct line needed to fix the warning

================================================================================= warnings summary =================================================================================
mne/conftest.py:1125
  /home/larsoner/python/mne-python/mne/conftest.py:1125: PytestRemovedIn9Warning: Marks applied to fixtures have no effect
  See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function
    @pytest.mark.filterwarnings("ignore:.*Extraction of measurement.*:")

Happy to add to some test if people think this is a good idea and will work in the general case. Happy to explore if it will work in some cases (esp. if someone can point me to the right place to add a test!).

Can also rebase and re-target for main but on my machine at least I get a cryptic error when I try to use latest main with MNE-Python so haven't done that for now.

@larsoner larsoner marked this pull request as draft February 20, 2024 17:00
@larsoner larsoner changed the base branch from 8.0.x to main February 20, 2024 17:15
@larsoner larsoner marked this pull request as ready for review February 20, 2024 17:15
@larsoner
Copy link
Contributor Author

Okay added a test, rebased, and targeted main so should be good to go from my end!

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

lovely enhancement 👍
please add a changelog fragment for towncrier, you can use the pr id as issue id

also do you want to rebase/squash or should we

@larsoner
Copy link
Contributor Author

Stub added, feel free to squash at your end!

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

Thanks so much @larsoner!

@Zac-HD Zac-HD merged commit 1640f2e into pytest-dev:main Feb 23, 2024
23 checks passed
@larsoner larsoner deleted the stacklevel branch February 23, 2024 06:16
flying-sheep pushed a commit to flying-sheep/pytest that referenced this pull request Apr 9, 2024
* ENH: Improve warning stacklevel

* TST: Add test

* TST: Ping

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* MAINT: Changelog

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants