kymo: make single line scan return a valid `line_time_seconds` #576

JoepVanlier · 2023-09-26T15:41:21Z

Why this PR?
Prior to this change, Kymos consisting of a single or partial scan line provide no line_time_seconds which prevents plotting and other functionality. Now they provide the line time of what is actually shown in the plot (e.g. a full line, without deadtime if the line time is smaller or equal than a single line).

The second issue is that for a few Bluelake versions, Bluelake put some kymographs that were taken with a fixed line count in the Scan field. The last commit contains a fallback method so that they will not be loaded as corrupted Scan items, but end up in the kymos field.

rpauszek

good stuff, it's nice to handle our mistakes graciously 😅 just a left a couple minor comments for clarity

rpauszek · 2023-10-31T14:57:27Z

changelog.md

@@ -7,6 +7,12 @@
 * Added more options for plotting color channels for images:
  * Shortcuts `"r"`, `"g"`, and `"b"` can now be used for plotting single color channels in addition to `"red"`, `"green"`, and `"blue"`.
  * Two-channel visualizations can be plotted using `"rg"`, `"gb"`, or `"rb"`.
+* Handle kymographs that were accidentally stored in the `Scan` field.


Suggested change

* Handle kymographs that were accidentally stored in the `Scan` field.

* Handle kymographs that were erroneously stored in the `Scan` field.

Is this fixed in BL and if so do we know in which version? Also, I think this would be more appropriate under Bug Fixes or Improvements

Good call on adding the version number!

And yeah that's fair, it's an improvement. I feel it's not necessarily a bugfix on pylake's part.

rpauszek · 2023-10-31T14:59:45Z

changelog.md

+
+#### Improvements
+
+* Kymographs that only consist of a single line no longer raise, but report a `line_time_seconds`. This also allows you to now call `Kymo.plot()` on these.


Suggested change

* Kymographs that only consist of a single line no longer raise, but report a `line_time_seconds`. This also allows you to now call `Kymo.plot()` on these.

* Kymographs consisting of a single scan line now return a valid `line_time_seconds`. This allows certain downstream functionality, such as `Kymo.plot()`.

rpauszek · 2023-10-31T15:06:25Z

lumicks/pylake/detail/image.py

-    )
+    pixel_boundary = np.argmax(infowave == InfowaveCode.pixel_boundary)
+    if infowave[pixel_boundary] != InfowaveCode.pixel_boundary:
+        raise RuntimeError("No completed pixel found in Scan")


Suggested change

raise RuntimeError("No completed pixel found in Scan")

raise RuntimeError("No completed pixel found in image")

maybe don't use "Scan" so as not to confuse with the class

rpauszek · 2023-10-31T15:29:16Z

lumicks/pylake/kymo.py


            # Special case for no dead time. Is it really no deadtime, or a single line?
            if dead_time == 0 and not np.any(beyond_first_line):
-                raise RuntimeError(single_line_error)
+                return scan_time * infowave._src.dt * ns_to_sec

        except ValueError:  # ValueError occurs when beyond_first_line is empty
-            raise RuntimeError(single_line_error)
+            return scan_time * infowave._src.dt * ns_to_sec


I can't suggest on the whole block, but it seems that the try ... except was there mostly to re-throw a more descriptive error. Since we don't need to do that now, we could move the early out up and save some indentation

Personally, I find it cleaner, for lines 35-44:

beyond_first_line = infowave.data[start + scan_time :] != InfowaveCode.discard if not np.any(beyond_first_line): return scan_time * infowave._src.dt * ns_to_sec # Special case for no dead time. Is it really no deadtime, or a single line? if (dead_time := np.argmax(beyond_first_line)) == 0: return scan_time * infowave._src.dt * ns_to_sec

I tried it and all of the tests pass

Yeah, that's fair. I'd rather check for len(beyond_first_line) than using np.any, since that's the only case we need to be covering (to prevent argmax from getting an empty array).

rpauszek · 2023-10-31T15:30:34Z

lumicks/pylake/kymotracker/detail/msd_estimation.py

+        if len(coordinate) <= 4:
+            raise RuntimeError(
+                "You need at least 5 time points to estimate the number of points to include in "
+                "the fit."
+            )


don't we have this check already somewhere?

We do have a similar check elsewhere, but you don't even get to that check if you run diffusion estimation on an empty track. Instead you are greeted by a TypeError: expected non-empty vector for x which is not helpful.

prior to this change, kymographs with a single line could not be plotted since `line_time_seconds` is unavailable

JoepVanlier changed the title ~~Single line scan~~ kymo: make single line scan return a valid line_time_seconds Sep 26, 2023

rpauszek force-pushed the rework_confocal_tests branch 2 times, most recently from 66a2d99 to d774ff7 Compare September 27, 2023 09:41

Base automatically changed from rework_confocal_tests to main September 27, 2023 10:54

JoepVanlier force-pushed the single_line_scan branch from cd02fcc to 8868688 Compare September 27, 2023 14:11

JoepVanlier force-pushed the single_line_scan branch 2 times, most recently from 679b129 to 83818ad Compare October 24, 2023 14:13

JoepVanlier marked this pull request as ready for review October 24, 2023 15:25

JoepVanlier requested review from a team as code owners October 24, 2023 15:25

JoepVanlier requested review from tobiasjj and rpauszek October 24, 2023 15:25

JoepVanlier force-pushed the single_line_scan branch 2 times, most recently from a9c3ff4 to 6652798 Compare October 27, 2023 14:11

rpauszek approved these changes Oct 31, 2023

View reviewed changes

JoepVanlier added 3 commits October 31, 2023 17:21

kymo: support kymographs with a single line

0d81401

prior to this change, kymographs with a single line could not be plotted since `line_time_seconds` is unavailable

diffusion: better error for "ols" of empty track

fa1b384

file: support loading a misclassified Kymo

319d50a

JoepVanlier force-pushed the single_line_scan branch from 6652798 to 319d50a Compare October 31, 2023 16:37

JoepVanlier merged commit 5588392 into main Oct 31, 2023
8 checks passed

JoepVanlier deleted the single_line_scan branch October 31, 2023 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kymo: make single line scan return a valid `line_time_seconds` #576

kymo: make single line scan return a valid `line_time_seconds` #576

JoepVanlier commented Sep 26, 2023 •

edited

Loading

rpauszek left a comment

rpauszek Oct 31, 2023

JoepVanlier Oct 31, 2023

rpauszek Oct 31, 2023

rpauszek Oct 31, 2023

rpauszek Oct 31, 2023

JoepVanlier Oct 31, 2023

rpauszek Oct 31, 2023

JoepVanlier Oct 31, 2023

	* Handle kymographs that were accidentally stored in the `Scan` field.
	* Handle kymographs that were erroneously stored in the `Scan` field.


		#### Improvements

		* Kymographs that only consist of a single line no longer raise, but report a `line_time_seconds`. This also allows you to now call `Kymo.plot()` on these.

	* Kymographs that only consist of a single line no longer raise, but report a `line_time_seconds`. This also allows you to now call `Kymo.plot()` on these.
	* Kymographs consisting of a single scan line now return a valid `line_time_seconds`. This allows certain downstream functionality, such as `Kymo.plot()`.

	raise RuntimeError("No completed pixel found in Scan")
	raise RuntimeError("No completed pixel found in image")

kymo: make single line scan return a valid line_time_seconds #576

kymo: make single line scan return a valid line_time_seconds #576

Conversation

JoepVanlier commented Sep 26, 2023 • edited Loading

rpauszek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kymo: make single line scan return a valid `line_time_seconds` #576

kymo: make single line scan return a valid `line_time_seconds` #576

JoepVanlier commented Sep 26, 2023 •

edited

Loading