napari / napari

napari: a fast, interactive, multi-dimensional image viewer for python

https://napari.org

BSD 3-Clause "New" or "Revised" License

2.08k stars 411 forks source link

Add mechanism for inheritance of `affine`, `rotate`, `scale`, `shear`, `translate` #6827

Open Czaki opened 1 month ago

Czaki commented 1 month ago

References and relevant issues

extracted from #6780

Description

This PR infroduces:

1) SampleLayer - simple layer to simplify and speedup testing that does not depend on given layer type. It is minimum class inheriting from Layer 2) add parameters_with_default_values property to Layer and LayerList to hold information which properties have default values. 3) inheritance for affine, rotate, scale, translate (I need to dive deeper in shear). 4) Allow set item in TransformChain by name, not index (I may extract it to separate PR).

This PR adds a basic test. It will be improved If the whole shape looks good.

brisvag commented 1 month ago

Could you explain a bit the background of this feature, what problem it tries to solve?

Czaki commented 1 month ago

Could you explain a bit the background of this feature, what problem it tries to solve?

If one layer does not have some parameters set, then try to guess them to keep layers in same coordinate space.

For example:

put points loaded from csv in same space as image is - https://napari.zulipchat.com/#narrow/stream/212875-general/topic/Points.20layer.20and.20layer.2Escale.20from.20image.20layer/near/432068947
load labels saved as NumPy array into the same coordinate space as the earlier loaded image.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.49%. Comparing base (36bb0d3) to head (7c43158).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #6827 +/- ## ========================================== + Coverage 92.45% 92.49% +0.04% ========================================== Files 617 618 +1 Lines 55161 55484 +323 ========================================== + Hits 50998 51319 +321 - Misses 4163 4165 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

andy-sweet commented 2 weeks ago

4. Allow set item in TransformChain by name, not index (I may extract it to separate PR).

I haven't taken a proper look at this PR, but as a minor bonus, this fixes the almost 3-year old https://github.com/napari/napari/issues/3058

In the discussion there, we suggested removing the lookup by name feature entirely, but none of us felt too strongly. Given that supporting setting by name is easy, I think the addition here is also fine. Though I'd maybe expect the functionality to be in TransformChain rather than EventedList especially as psygnal's EventedList does not support this functionality. None of this is blocking, but hopefully is useful information.