napari / napari

napari: a fast, interactive, multi-dimensional image viewer for python
https://napari.org
BSD 3-Clause "New" or "Revised" License
2.07k stars 410 forks source link

Split points into `Points` and `BasePoints` in preparation for Graph layer #6867

Open DragaDoncila opened 3 weeks ago

DragaDoncila commented 3 weeks ago

References and relevant issues

This PR is a precursor to implementing the GraphLayer - see #5861 for a WIP on the graph layer itself.

Description

In discussions during the Graph Layer working group we found that getting the PR close to review was very challenging due to frequent merge conflicts with the Points layer, which were not trivial to resolve because that PR currently makes the split you see in this PR. We therefore decided to bring this change into its own PR.

This PR therefore splits the current Points layer functionality into a _BasePoints and Points layer. This allows the GraphLayer to reuse this code to represent its nodes.

Summary of Key Changes:

data and _points_data

Previously, the list of point coordinates lived at Points.data and self.data was used throughout Points as needed. In this PR, _BasePoints uses a new property, _points_data for all implemented methods.

Points overrides this property to return self.data. This means everything that's in the base class works via _points_data, and can be reused by Graph, but Points still keeps its public API and data setters mostly the same.

_BasePoints.data getter, _BasePoints._points_data and _BasePoints._set_data are all abstract. The _BasePoints.data setter is fully implemented i.e. copied from Points, and Points uses it explicitly via _BasePoints.data.fset(self, data).

Other abstract methods on _BasePoints

Extracted from Points to _BasePoints

Remains in Points only

...

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 97.52747% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 92.44%. Comparing base (5527240) to head (551c7c0). Report is 3 commits behind head on main.

Files Patch % Lines
napari/layers/points/_base.py 97.47% 18 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6867 +/- ## ========================================== + Coverage 92.43% 92.44% +0.01% ========================================== Files 614 618 +4 Lines 54903 55201 +298 ========================================== + Hits 50748 51031 +283 - Misses 4155 4170 +15 ```

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

Czaki commented 2 weeks ago

Please do not forget to include similar test for a new class.

https://github.com/napari/napari/blob/f66fce680632a1e9b78f73b00863f5ad8ab88d1d/napari/layers/points/_tests/test_points.py#L2660-L2663