napari / napari-animation

A napari plugin for making animations
https://napari.github.io/napari-animation/
Other
74 stars 27 forks source link

Fix interpolation problem with ints #189

Closed Czaki closed 7 months ago

Czaki commented 7 months ago

This PR changes interpolate_numbehaviour to fallback to float like a number if the first argument is Interger and the second one is Real.

closes #149 closes #152

Czaki commented 7 months ago

Doiesn't fully solve these issue, right? If you have 2 keyframes and opacity goes from 1 to 2 and they're both integers, you'll get a flip in the middle no?

No. This will still return int (so 1 or 2 in this case). There are scenarios when interpolating int should return int (for example for 0, 100, and fraction 0.5 the result should be 50, not 50.0).

brisvag commented 7 months ago

Yes, but 1 and 2 from a "float" field should return 1.5 at fraction 0.75, not 1. My point being, we should still enforce types in napari.

Czaki commented 7 months ago

Yes, but 1 and 2 from a "float" field should return 1.5 at fraction 0.75, not 1. My point being, we should still enforce types in napari.

I totally agree. This is only a fix to reduce the probability of the problem. As it may be hard to find every place that needs coerce and ensure that any future PR will not introduce new problematic property.

codecov[bot] commented 7 months ago

Codecov Report

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

Comparison is base (23c1a3c) 85.91% compared to head (40518ab) 86.06%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #189 +/- ## ========================================== + Coverage 85.91% 86.06% +0.14% ========================================== Files 26 26 Lines 1051 1062 +11 ========================================== + Hits 903 914 +11 Misses 148 148 ```

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