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 size and ndim to LayerDataProtocol #6494

Merged
merged 1 commit into from Jan 8, 2024
Merged

Add size and ndim to LayerDataProtocol #6494

merged 1 commit into from Jan 8, 2024

Conversation

Carreau
Copy link
Contributor

@Carreau Carreau commented Nov 23, 2023

in napari/layers/image/image.py, _calc_data_range(self...) calls calc_data_range(input_data,...) where input_data is a LayerDataProtocol

calc_data_range(...) needs it to have a size, and ndim. So my guess is we need to make sure that LayerDataProtocol has size and ndim, is that correct ?

Thoughts ?

Typing all the things that need to be types for this problem to show up is quite long, please see all my other typing PRs to start enabling type checking for these files, mostly opening this separately to know if this is something we wan to add to LayerDataProtocol

@Carreau
Copy link
Contributor Author

Carreau commented Nov 23, 2023

add the corresponding (size, ndim) properties to MultiScaleData.

Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (93af19a) 92.24% compared to head (f3fba67) 92.19%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6494      +/-   ##
==========================================
- Coverage   92.24%   92.19%   -0.05%     
==========================================
  Files         601      601              
  Lines       53210    53236      +26     
==========================================
- Hits        49081    49079       -2     
- Misses       4129     4157      +28     

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

Copy link
Collaborator

@Czaki Czaki left a comment

Choose a reason for hiding this comment

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

Change to raise NotImplementedError() or add pass to coverage:report exclude_lines in setup.cfg

napari/layers/_data_protocols.py Outdated Show resolved Hide resolved
napari/layers/_data_protocols.py Outdated Show resolved Hide resolved
in `napari/layers/image/image.py`, `_calc_data_range(self...)` calls
`calc_data_range(input_data,...)` where `input_data` is a `LayerDataProtocol`

`calc_data_range(...)` needs it to have a size, and ndim. So my guess is
we need to make sure that LayerDataProtocol has size and ndim, is that
correct ?
@Czaki Czaki added maintenance PR with maintance changes, ready to merge Last chance for comments! Will be merged in ~24h typing PR that focus on typing improvement labels Nov 24, 2023
@Czaki Czaki added this to the 0.5.0 milestone Nov 24, 2023
@jni jni merged commit a7c01c0 into napari:main Jan 8, 2024
35 of 38 checks passed
@jni jni removed the ready to merge Last chance for comments! Will be merged in ~24h label Jan 8, 2024
kne42 added a commit to kne42/napari that referenced this pull request Jan 11, 2024
* main:
  Fix labels mapping cache by filling it with background, not 0 (napari#6580)
  Simplify unused parameters of Quaternion functions. (napari#6424)
  Add size and ndim to LayerDataProtocol (napari#6494)
  Fix label direct mode for installation without numba (napari#6571)
  Fix test in napari_builtins to remove import from conftest (napari#6568)
  Remove `app-model!=0.2.4` from test constraints (napari#6577)
  Bump mypy version and fix errors (napari#6557)
  Update test to work with `app-model==0.2.4` (napari#6573)
  Added support for features in surface layers (napari#6515)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance PR with maintance changes, typing PR that focus on typing improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants