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

Add typing to Layer.__init__ #6796

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Conversation

dstansby
Copy link
Contributor

Some more typing - there were also a couple of code fixes I had to make after adding the typing.

Copy link

codecov bot commented Mar 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (8f8d420) to head (9e1ad0b).
Report is 62 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6796      +/-   ##
==========================================
+ Coverage   92.41%   92.43%   +0.02%     
==========================================
  Files         615      614       -1     
  Lines       54892    55170     +278     
==========================================
+ Hits        50729    50998     +269     
- Misses       4163     4172       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dstansby dstansby marked this pull request as ready for review March 30, 2024 16:30
Copy link
Member

@psobolewskiPhD psobolewskiPhD left a comment

Choose a reason for hiding this comment

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

Small suggestion

napari/types.py Outdated Show resolved Hide resolved
napari/types.py Outdated Show resolved Hide resolved
napari/layers/base/base.py Outdated Show resolved Hide resolved
napari/layers/base/base.py Outdated Show resolved Hide resolved
psobolewskiPhD and others added 3 commits April 6, 2024 17:45
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
@@ -306,25 +307,29 @@ class Layer(KeymapProvider, MousemapProvider, ABC, metaclass=PostInit):

def __init__(
self,
data,
ndim,
data: Union[npt.NDArray, list[npt.NDArray]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be also dask, zarr, tensorstore.

translate=None,
visible=True,
):
affine: Optional[Union[npt.NDArray, Affine]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
affine: Optional[Union[npt.NDArray, Affine]] = None,
affine: Union[npt.NDArray, Affine, None] = None,

napari/layers/base/base.py Outdated Show resolved Hide resolved
Comment on lines +316 to +318
experimental_clipping_planes: Optional[
Union[list[dict], list[ClippingPlane], ClippingPlaneList]
] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
experimental_clipping_planes: Optional[
Union[list[dict], list[ClippingPlane], ClippingPlaneList]
] = None,
experimental_clipping_planes: Union[list[dict], list[ClippingPlane], ClippingPlaneList, None] = None,

rotate: Optional[
Union[float, tuple[float, float, float], npt.NDArray]
] = None,
scale: Optional[tuple[float, ...]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
scale: Optional[tuple[float, ...]] = None,
scale: Optional[Sequence[float, ...]] = None,

] = None,
scale: Optional[tuple[float, ...]] = None,
shear: Optional[npt.NDArray] = None,
translate: Optional[tuple[float, ...]] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
translate: Optional[tuple[float, ...]] = None,
translate: Optional[Sequence[float, ...]] = None,

@@ -596,7 +596,7 @@ def compute_multiscale_level_and_corners(


def coerce_affine(
affine: Union[npt.ArrayLike, Affine],
affine: Optional[Union[npt.ArrayLike, Affine]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
affine: Optional[Union[npt.ArrayLike, Affine]],
affine: Union[npt.ArrayLike, Affine, None],

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
@GenevieveBuckley
Copy link
Contributor

The mypy CI check is currently failing due to the new napari/layers/base/_test_util_sample_layer.py file introduced in the main branch:

 napari/layers/base/_test_util_sample_layer.py:48: error: Unused "type: ignore" comment  [unused-ignore]
Found 1 error in 1 file (checked 667 source files)

Two options to fix this:

  1. Move this file into a _tests module (so mypy will ignore it), and update the import statement in test_qaction_layer.py to match. Eg: napari/layers/base/_test_util_sample_layer.py -> 'napari/layers/base/_tests/_test_util_sample_layer.py'. It's a test file, so I'm a bit confused about why it's living outside a test folder to begin with anyway.
  2. OR, merge main into this PR branch, then remove the comment # type: ignore [no-untyped-call] from line 48 of _test_util_sample_layer.py

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.

None yet

4 participants