1.) Provenance logging refactor and support for SpotFindingResults #1517

shanaxel42 · 2019-08-29T07:30:04Z

This PR does a bit of a provenance log refactor fixing some issues and also setting things up to work with the new FindSpots and DecodeSpots modules.

Depends on #1515 #1489 #1516

codecov-io · 2019-08-29T07:40:03Z

Codecov Report

Merging #1517 into master will decrease coverage by 0.07%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #1517      +/-   ##
==========================================
- Coverage   87.54%   87.47%   -0.08%     
==========================================
  Files         145      145              
  Lines        5036     5053      +17     
==========================================
+ Hits         4409     4420      +11     
- Misses        627      633       +6

Impacted Files	Coverage Δ
starfish/test/full_pipelines/api/test_dartfish.py	`100% <100%> (ø)`	⬆️
starfish/core/imagestack/imagestack.py	`94.39% <100%> (-0.05%)`	⬇️
starfish/core/util/logging.py	`97.5% <100%> (+0.83%)`	⬆️
starfish/test/full_pipelines/api/test_iss_api.py	`100% <100%> (ø)`	⬆️
starfish/core/types/_spot_finding_results.py	`54.83% <66.66%> (+0.99%)`	⬆️
starfish/core/pipeline/algorithmbase.py	`87.87% <77.77%> (-12.13%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af8af16...8338ae1. Read the comment docs.

ambrosejcarr · 2019-09-13T16:22:41Z

starfish/core/util/logging.py

+class Log:
+    def __init__(self):
+        """
+        Small class for capturing methods applied to throughout a pipeline.


Suggested change

Small class for capturing methods applied to throughout a pipeline.

Class for capturing methods and their parameters applied by an analysis pipeline.

ambrosejcarr · 2019-09-13T18:08:17Z

starfish/core/util/logging.py

+from starfish.core.types import CORE_DEPENDENCIES, LOG
+
+
+class Log:


Love the idea. Is this only used by spot finding? My intuition is that this should trigger a bigger refactor.

Nah this is used by everything. This PR was basically the "bigger" logging refactor needed to keep everything working throughout the refactor

ambrosejcarr · 2019-09-13T18:09:24Z

starfish/core/pipeline/algorithmbase.py

+                    spot_results = kwargs['spots']
+                    spot_results.log.update_log(args[0])
+                    result.attrs[STARFISH_EXTRAS_KEY] = spot_results.log.encode()
+                # OLD CODE FOR DETECT WILL GET REMOVED


Suggested change

# OLD CODE FOR DETECT WILL GET REMOVED

# TODO shanaxel42 OLD CODE FOR DETECT WILL GET REMOVED

ttung · 2019-09-25T23:19:44Z

starfish/core/pipeline/algorithmbase.py

-                    it.attrs[STARFISH_EXTRAS_KEY] = LogEncoder().encode({LOG: stack.log})
+            if result is not None:
+                method_class_str = str(args[0].__class__)
+                if 'ApplyTransform' in method_class_str or 'Filter' in method_class_str:


It's plausible that we have mechanisms that take multiple loggable objects.

It might be a good idea to have a base ProvenanceTracked class that all objects (ImageStack, IntensityTable, etc.) inherit, and then to walk through all the arguments to find objects that extend ProvenanceTracked. Then scrape the logs from those objects and build up a provenance graph.

this seems like a larger refactor, like maybe something post spot finding stuff

Sure, can you tack that onto the logging epic?

There are a lot of gaps in the logging -- for instance, ApplyTransform plausibly should log the origin of the warping data. Also can be covered afterwards.

starfish/core/util/logging.py

ttung · 2019-10-03T16:09:27Z

starfish/core/pipeline/algorithmbase.py

-                    it.attrs[STARFISH_EXTRAS_KEY] = LogEncoder().encode({LOG: stack.log})
+            if result is not None:
+                method_class_str = str(args[0].__class__)
+                if 'ApplyTransform' in method_class_str or 'Filter' in method_class_str:


Sure, can you tack that onto the logging epic?

There are a lot of gaps in the logging -- for instance, ApplyTransform plausibly should log the origin of the warping data. Also can be covered afterwards.

This was referenced Aug 29, 2019

Implement SpotFinding Refactor #1514

Closed

2) ISS spot finding refactor #1518

Merged

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 87cd345 to a9f9535 Compare August 29, 2019 16:28

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 2 times, most recently from 69e0b02 to 5eae3cc Compare August 29, 2019 16:46

shanaxel42 requested review from ttung and ambrosejcarr August 29, 2019 17:00

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 18c85b6 to 29d66f5 Compare August 30, 2019 17:51

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from ad12586 to a484e73 Compare August 30, 2019 18:31

ambrosejcarr changed the title ~~Provancene logging refactor and support for SpotFindingResults~~ Provenance logging refactor and support for SpotFindingResults Sep 13, 2019

ambrosejcarr reviewed Sep 13, 2019

View reviewed changes

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 29d66f5 to 9bf4e8e Compare September 17, 2019 21:51

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from a484e73 to 4b53ae7 Compare September 17, 2019 21:51

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 9bf4e8e to b5a67db Compare September 17, 2019 23:42

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 2 times, most recently from ed717ad to 8cb311e Compare September 18, 2019 18:51

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from b5a67db to 699ec22 Compare September 18, 2019 18:51

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from 8cb311e to d7c7372 Compare September 18, 2019 19:30

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 189483f to 94befac Compare September 18, 2019 20:27

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from d7c7372 to 4c3fc56 Compare September 18, 2019 20:27

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 94befac to af7cd8a Compare September 18, 2019 20:44

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from 4c3fc56 to 82b52da Compare September 18, 2019 20:44

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from af7cd8a to c27f333 Compare September 18, 2019 21:04

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 4 times, most recently from ed7b64d to 4e1c009 Compare September 19, 2019 18:04

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 71e65d9 to 5ae5c14 Compare September 19, 2019 21:09

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from 4e1c009 to 08386dc Compare September 19, 2019 21:09

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 2 times, most recently from 29bcddc to 99b64d8 Compare September 19, 2019 22:12

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 3664502 to e9a2178 Compare September 19, 2019 23:03

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from 99b64d8 to 4db2a9b Compare September 19, 2019 23:03

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from e9a2178 to 9147e3a Compare September 19, 2019 23:42

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 3 times, most recently from aadd9a8 to b214df1 Compare September 20, 2019 15:27

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from 1d04269 to 5589427 Compare September 20, 2019 20:59

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 2 times, most recently from 4ce1efd to debb55b Compare September 23, 2019 18:06

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch 2 times, most recently from cbcdcb6 to dae77b7 Compare September 23, 2019 20:00

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch 2 times, most recently from 4b3fa52 to f090a90 Compare September 23, 2019 22:18

shanaxel42 force-pushed the saxelrod-add_coordinates-to-spot-results branch from dae77b7 to 403b0bd Compare September 23, 2019 22:18

shanaxel42 changed the base branch from saxelrod-add_coordinates-to-spot-results to master September 24, 2019 16:46

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from f090a90 to bf35640 Compare September 24, 2019 16:49

ttung force-pushed the master branch from 67a6870 to 849058e Compare September 24, 2019 18:10

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from bf35640 to c220e44 Compare September 25, 2019 20:36

ttung reviewed Sep 25, 2019

View reviewed changes

shanaxel42 changed the title ~~Provenance logging refactor and support for SpotFindingResults~~ 1.) Provenance logging refactor and support for SpotFindingResults Oct 1, 2019

provanance logging refactor and support for SpotFindingResults

8338ae1

shanaxel42 force-pushed the saxelrod-add-logging-support-spot-results branch from c220e44 to 8338ae1 Compare October 1, 2019 14:16

This was referenced Oct 1, 2019

3.) SeqFish/StarMap/LocalBlobSearch #1592

Merged

4.) refactoring allen smFish with new spot finding #1593

Merged

5.) refactoring localMaxPeakFinder/osmFIsh #1594

Merged

6.)adding MetricDecoder to DecodeSpots #1596

Merged

ttung approved these changes Oct 3, 2019

View reviewed changes

shanaxel42 merged commit 912d176 into master Oct 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1.) Provenance logging refactor and support for SpotFindingResults #1517

1.) Provenance logging refactor and support for SpotFindingResults #1517

shanaxel42 commented Aug 29, 2019 •

edited

Loading

codecov-io commented Aug 29, 2019 •

edited

Loading

ambrosejcarr Sep 13, 2019

ambrosejcarr Sep 13, 2019

shanaxel42 Sep 17, 2019

ambrosejcarr Sep 13, 2019

ttung Sep 25, 2019

shanaxel42 Sep 26, 2019

ttung Oct 3, 2019

ttung Oct 3, 2019

	Small class for capturing methods applied to throughout a pipeline.
	Class for capturing methods and their parameters applied by an analysis pipeline.

		from starfish.core.types import CORE_DEPENDENCIES, LOG


		class Log:

	# OLD CODE FOR DETECT WILL GET REMOVED
	# TODO shanaxel42 OLD CODE FOR DETECT WILL GET REMOVED

1.) Provenance logging refactor and support for SpotFindingResults #1517

1.) Provenance logging refactor and support for SpotFindingResults #1517

Conversation

shanaxel42 commented Aug 29, 2019 • edited Loading

codecov-io commented Aug 29, 2019 • edited Loading

Codecov Report

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

shanaxel42 commented Aug 29, 2019 •

edited

Loading

codecov-io commented Aug 29, 2019 •

edited

Loading