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

[pre-commit.ci] pre-commit autoupdate #5645

Merged
merged 9 commits into from Apr 17, 2023
Merged

Conversation

pre-commit-ci[bot]
Copy link
Contributor

@pre-commit-ci pre-commit-ci bot commented Mar 20, 2023

@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #5645 (558ca48) into main (e12acd8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##             main    #5645    +/-   ##
========================================
  Coverage   89.84%   89.84%            
========================================
  Files         608      608            
  Lines       51693    51876   +183     
========================================
+ Hits        46441    46607   +166     
- Misses       5252     5269    +17     
Impacted Files Coverage Δ
napari/__main__.py 44.49% <ø> (ø)
napari/components/_tests/test_multichannel.py 100.00% <ø> (ø)
napari/layers/image/image.py 95.70% <ø> (-0.02%) ⬇️
napari/_app_model/context/_context.py 59.45% <100.00%> (-1.07%) ⬇️
napari/_qt/dialogs/qt_package_installer.py 87.07% <100.00%> (-0.05%) ⬇️
napari/_qt/qt_main_window.py 74.17% <100.00%> (-0.05%) ⬇️
napari/_tests/test_magicgui.py 96.65% <100.00%> (ø)
napari/_tests/test_view_layers.py 98.30% <100.00%> (-0.02%) ⬇️
...i/layers/points/_tests/test_points_key_bindings.py 100.00% <100.00%> (ø)
napari/plugins/__init__.py 94.28% <100.00%> (-0.16%) ⬇️
... and 5 more

... and 32 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Czaki
Copy link
Collaborator

Czaki commented Mar 20, 2023

The fail comes from this rule: https://beta.ruff.rs/docs/rules/no-explicit-stacklevel/

Carreau added a commit to Carreau/napari that referenced this pull request Mar 21, 2023
Step toward napari#5645 only touching a few file to make review easier
@pre-commit-ci pre-commit-ci bot force-pushed the pre-commit-ci-update-config branch from e7f7b91 to 46fdfb4 Compare March 27, 2023 20:13
@github-actions github-actions bot added the tests Something related to our tests label Mar 27, 2023
@pre-commit-ci pre-commit-ci bot force-pushed the pre-commit-ci-update-config branch from 7acad91 to eaa28a8 Compare April 3, 2023 21:14
Comment on lines 10 to 24
Viewer = ...
current_viewer = ...

gui_qt = napari._qt.qt_event_loop.gui_qt
run = napari._qt.qt_event_loop.run
save_layers = napari.plugins.io.save_layers
gui_qt = ...
run = ...
save_layers = ...

view_image = napari.view_layers.view_image
view_labels = napari.view_layers.view_labels
view_path = napari.view_layers.view_path
view_points = napari.view_layers.view_points
view_shapes = napari.view_layers.view_shapes
view_surface = napari.view_layers.view_surface
view_tracks = napari.view_layers.view_tracks
view_vectors = napari.view_layers.view_vectors
view_image = ...
view_labels = ...
view_path = ...
view_points = ...
view_shapes = ...
view_surface = ...
view_tracks = ...
view_vectors = ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

this changes are caused by https://beta.ruff.rs/docs/rules/#flake8-pyi-pyi PYI015 that is implementation of rule Y015 form https://pypi.org/project/flake8-pyi/

At this moment I do not understand the motivation for this change. But will try to investigate this.

But in general, I think this change should be reverted, and maybe just rules skipped.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does not seem like a bad rule to me in general, I just don't understand the fix O.o

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just use implicit reexport https://mypy.readthedocs.io/en/stable/config_file.html#confval-implicit_reexport that is treated as valid by mypy so do not changed by ruff

It was also mentioned here https://scientific-python.org/specs/spec-0001/#type-checkers

It was split on multiple lines because ruff configuration combine-as-imports is set by default to false.

Carreau added a commit to Carreau/napari that referenced this pull request Apr 4, 2023
Step toward napari#5645 only touching a few file to make review easier
Carreau added a commit to Carreau/napari that referenced this pull request Apr 4, 2023
Step toward napari#5645 only touching a few file to make review easier
@Czaki Czaki force-pushed the pre-commit-ci-update-config branch from 096af54 to dcb4521 Compare April 5, 2023 21:21
updates:
- [github.com/psf/black: 23.1.0 → 23.3.0](psf/black@23.1.0...23.3.0)
- [github.com/charliermarsh/ruff-pre-commit: v0.0.256 → v0.0.261](astral-sh/ruff-pre-commit@v0.0.256...v0.0.261)
@pre-commit-ci pre-commit-ci bot force-pushed the pre-commit-ci-update-config branch from dcb4521 to efa85f5 Compare April 10, 2023 20:23
Czaki pushed a commit that referenced this pull request Apr 13, 2023
Step toward #5645 only touching a few file to make review easier

New pre-commit configuration (new version of ruff) requires warnings to
have a stacklevel.
Comment on lines 5 to 7
from napari.view_layers import (
view_image as view_image,
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from napari.view_layers import (
view_image as view_image,
)
from napari.view_layers import view_image

??? I don't understand this at all

Copy link
Member

Choose a reason for hiding this comment

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

(Sorry, I missed the above links because they related to a different kind of change. But I still don't like this...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. The currently this syntax is enforced by "PYI015" https://beta.ruff.rs/docs/rules/#flake8-pyi-pyi rule of ruff.
Unfortunately, this rule does not accept __all__ even when mypy accepts it.

But the current code was wrong as it does not contain explicit reexport or __all__.

I'm open to disabling "PYI015". But I also see profit from enforcing this rule. It will be nice to hear from other core devs (I will merge this PR on Sunday, but if there is no feedback immediately will open a followup).

Copy link
Member

Choose a reason for hiding this comment

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

But I also see profit from enforcing this rule.

Such as...? 😂 I prefer __all__ by a significant margin... As in my other reviews... Code should be obvious in what it does... I suspect I would not be the first person to be totally baffled by these lines. I also think that we should combine the import lines so that we don't have so many

from X import (
    Y as Y
)

It's really quite horrible imho!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Multiple import as comes from ruff configuration https://beta.ruff.rs/docs/settings/#combine-as-imports

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have combined imports. I will merge in about 12h to be before the automatic update. Then I will open the Issue for a longer discussion.

@github-actions github-actions bot added the qt Relates to qt label Apr 16, 2023
@Czaki Czaki merged commit f6f46e6 into main Apr 17, 2023
30 of 32 checks passed
@Czaki Czaki deleted the pre-commit-ci-update-config branch April 17, 2023 13:54
@Czaki Czaki mentioned this pull request Jun 7, 2023
@Czaki Czaki added this to the 0.4.18 milestone Jun 15, 2023
@Czaki Czaki added the maintenance PR with maintance changes, label Jun 15, 2023
@Czaki Czaki added the triaged-0.4.18 To mark PR that is triaged in 0.4.18 release process label Jun 15, 2023
Czaki pushed a commit that referenced this pull request Jun 19, 2023
Step toward #5645 only touching a few file to make review easier

New pre-commit configuration (new version of ruff) requires warnings to
have a stacklevel.
Czaki added a commit that referenced this pull request Jun 19, 2023
<!--pre-commit.ci start-->
updates:
- [github.com/psf/black: 23.1.0 →
23.3.0](psf/black@23.1.0...23.3.0)
- [github.com/charliermarsh/ruff-pre-commit: v0.0.256 →
v0.0.261](astral-sh/ruff-pre-commit@v0.0.256...v0.0.261)
<!--pre-commit.ci end-->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
Step toward #5645 only touching a few file to make review easier

New pre-commit configuration (new version of ruff) requires warnings to
have a stacklevel.
Czaki added a commit that referenced this pull request Jun 21, 2023
<!--pre-commit.ci start-->
updates:
- [github.com/psf/black: 23.1.0 →
23.3.0](psf/black@23.1.0...23.3.0)
- [github.com/charliermarsh/ruff-pre-commit: v0.0.256 →
v0.0.261](astral-sh/ruff-pre-commit@v0.0.256...v0.0.261)
<!--pre-commit.ci end-->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
Step toward #5645 only touching a few file to make review easier

New pre-commit configuration (new version of ruff) requires warnings to
have a stacklevel.
Czaki added a commit that referenced this pull request Jun 21, 2023
<!--pre-commit.ci start-->
updates:
- [github.com/psf/black: 23.1.0 →
23.3.0](psf/black@23.1.0...23.3.0)
- [github.com/charliermarsh/ruff-pre-commit: v0.0.256 →
v0.0.261](astral-sh/ruff-pre-commit@v0.0.256...v0.0.261)
<!--pre-commit.ci end-->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki pushed a commit that referenced this pull request Jun 21, 2023
Step toward #5645 only touching a few file to make review easier

New pre-commit configuration (new version of ruff) requires warnings to
have a stacklevel.
Czaki added a commit that referenced this pull request Jun 21, 2023
<!--pre-commit.ci start-->
updates:
- [github.com/psf/black: 23.1.0 →
23.3.0](psf/black@23.1.0...23.3.0)
- [github.com/charliermarsh/ruff-pre-commit: v0.0.256 →
v0.0.261](astral-sh/ruff-pre-commit@v0.0.256...v0.0.261)
<!--pre-commit.ci end-->

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance PR with maintance changes, qt Relates to qt tests Something related to our tests triaged-0.4.18 To mark PR that is triaged in 0.4.18 release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants