-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Format complete ISS experiment and expose in starfish.data #1316
Conversation
… experiment, update TileFetcher
Codecov Report
@@ Coverage Diff @@
## master #1316 +/- ##
===========================================
- Coverage 88.96% 69.05% -19.92%
===========================================
Files 146 148 +2
Lines 5258 5377 +119
===========================================
- Hits 4678 3713 -965
- Misses 580 1664 +1084
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
docs/source/_static/data_formatting_examples/format_iss_breast_data.py
Outdated
Show resolved
Hide resolved
self.coordinates_df.loc[fov, "y_min"], | ||
self.coordinates_df.loc[fov, "y_max"] | ||
), | ||
Coordinates.Z: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this experiment have multiple Z layers? If not, I think you can just omit this.
self.coordinates_df.loc[fov, "y_min"], | ||
self.coordinates_df.loc[fov, "y_max"] | ||
), | ||
Coordinates.Z: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
) | ||
|
||
|
||
if __name__ == "__main__": | ||
""" | ||
This TileFetcher should be run on data found at: | ||
s3://spacetx.starfish.data/mignardi_breast_1/raw/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to move this sucker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should move this in #1314
) | ||
|
||
|
||
if __name__ == "__main__": | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider moving this to be the description
argument passed to argparse.ArgumentParser()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want these scripts to be user-facing and user-friendly there's a lot of work to be done across them; right now we have a mixture of click and argparse scripts. If we think it's worth cleaning these up I'd suggest we do them all in a separate PR.
…_data.py Co-Authored-By: Tony Tung <tonytung@merly.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait for green and go
Testing strategy:
fov_000
in new experiment (is namedfov_001
in master)