Cleanup DisplacementField

[jankrepl]

Closes #9

What was kept

• __init__ - constructs the instance using two numpy arrays (dx, dy)
• __eq__ - checking equality - uses np.allclose with default parameters
• __mul__ - multiplying by a constant (mainly for visualization [(c * df).warp(img) for c in np.linspace(0, 1)])
• __rmul__ - see __mul__
• from_file - one can load a DF from a .npy lying on the disk
• from_transformation - Creating an instance via (tx, ty) (rather than (dx, dy) as in the __init__). Used in warp and sync.get_transform.
• norm - Compute norm. Very useful for visualizing since one can plt.imshow(df.norm) + one line implementation.
• save - Save to disk as a .npy file. Compatible with from_file.
• transformation - (tx, ty) Actual pixel positions (rather than displacements (dx, dy)). Used inside of warp.
• warp - warping

What was removed (together with corresponding tests)

• __call__ - composition of 2 fields
• adjust - "smoothen" a displacement field
• average_displacement - computes the mean norm over all pixels
• delta_x_scaled, delta_y_scaled - divides the displacements by the height (resp. width)
• generate - DF generating, note that affine and affine_simple are still accessible via standalone functions
• jacobian - estimates jacobian for each pixel (gives info on smoothness)
• is_valid - checking whether all elements of delta_x and delta_y are finite
• mask - masking chosen pixels to have 0 displacement
• n_pixels - counts the number of pixels
• outsiders - for each pixels determines whether it is mapped outside of the image
• plot_dvf - plot displacement vector field
• plot_outside - plots the outsiders
• plot_ranges - plot domain and the range of the mapping
• resize - resizing a displacement field
• resize_constant - resizing (another way) a displacement field
• warp_annotation - warping with nearest neighbors interpolation. One can simply call warp(interpolation="nearest") instead

Random comments

• After this change DisplacementField is just a very thin wrapper around OpenCV's remap. Of course there is always a possibility of switching to a different backend in the future.
• No more methods that do plotting -> there is no need to import matplotlib inside of atldld/base.py
• Coverage report after running all tests (not skipping marked tests as opposed to our CI)

pytest -m "" --cov-report=term-missing

---------- coverage: platform darwin, python 3.8.11-final-0 ----------
Name                     Stmts   Miss Branch BrPart  Cover   Missing
--------------------------------------------------------------------
src/atldld/__init__.py       2      0      0      0   100%
src/atldld/base.py         101      0     32      0   100%
src/atldld/cli.py           20     20      2      0     0%   17-61
src/atldld/sync.py         168     62     60      9    58%   61, 108, 125, 293, 398, 401, 411-416, 503-520, 605->607, 614, 656-698, 745-789
src/atldld/utils.py        116     25     46      5    74%   57, 110, 121, 126-139, 500, 520-540
src/atldld/version.py        1      0      0      0   100%
--------------------------------------------------------------------
TOTAL                      408    107    140     14    71%

[jankrepl]

@Stannislav @EmilieDel Feel free to check it out:) Btw I had to fork since I do not have the rights to push to remote. Also, I do not have the rights to assign reviewers:)

[EmilieDel]

Thanks a lot for doing this cleaning @jankrepl ! I like the plot_dvf method but it is maybe better to keep this DisplacementField class minimal!

[jankrepl]

Thanks a lot for doing this cleaning @jankrepl ! I like the plot_dvf method but it is maybe better to keep this DisplacementField class minimal!

I suggest we can always create a separate module visualization.py that implements it + other visualizations (feel free to create an issue). I kind of like the idea of avoiding the import of matplotlib inside of base.py. IMO it should be really just the most important functionality.

Also, as discussed many times IMO plot_dvf is very misleading and has a lot of scaling issues.

[Stannislav]

Nice, thanks for the cleaning!

I totally agree that matplotlib should only ever be imported in one module, namely in a dedicated visualisation / plot / whatever module. So while we can think about keeping the plotting routines I definitely think they're better off in a separate module instead of the DisplacementField class.

I used a very similar plot_dvf function in my notebooks, but the arguments of quiver were slightly different:

def plot_dvf(dvf, ax, n_skip=1):
    dx, dy = dvf[:, ::n_skip, ::n_skip]
    lengths = np.hypot(dx, dy)
    ax.quiver(dx, dy, lengths, cmap="binary")

maybe this is helpful. Probably one should still call ax.invert_yaxis() though.

A small note, mabe this can be done here as a "while I'm at it": in the norm proprty I think we could use the np.hypot function, which is made exactly for computing this kind of things.

[jankrepl]

Nice, thanks for the cleaning!

I totally agree that matplotlib should only ever be imported in one module, namely in a dedicated visualisation / plot / whatever module. So while we can think about keeping the plotting routines I definitely think they're better off in a separate module instead of the DisplacementField class.

I used a very similar plot_dvf function in my notebooks, but the arguments of quiver were slightly different:

def plot_dvf(dvf, ax, n_skip=1):
    dx, dy = dvf[:, ::n_skip, ::n_skip]
    lengths = np.hypot(dx, dy)
    ax.quiver(dx, dy, lengths, cmap="binary")

maybe this is helpful. Probably one should still call ax.invert_yaxis() though.

A small note, mabe this can be done here as a "while I'm at it": in the norm proprty I think we could use the np.hypot function, which is made exactly for computing this kind of things.

Wow, never heard of np.hypot! Is it faster than the more general norm? Btw it is clever to actually pass the norm of the vectors to the quiver plot. Anyway, I created a new issue #44.

[Stannislav]

Wow, never heard of np.hypot! Is it faster than the more general norm? Btw it is clever to actually pass the norm of the vectors to the quiver plot. Anyway, I created a new issue #44.

I hope that it is faster, but I haven't tested...

[jankrepl]

Wow, never heard of np.hypot! Is it faster than the more general norm? Btw it is clever to actually pass the norm of the vectors to the quiver plot. Anyway, I created a new issue #44.

I hope that it is faster, but I haven't tested...

I created an issue for this #45

Anyway, could one of you guys merge this branch into main?