Skip to content
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

Add Decoded Spot Table #1087

Merged
merged 3 commits into from
Mar 19, 2019
Merged

Add Decoded Spot Table #1087

merged 3 commits into from
Mar 19, 2019

Conversation

ambrosejcarr
Copy link
Member

Adds a DecodedSpotTable and some basic serialization tooling to address #1076. Users who want the underlying dataframe can get it with DecodedSpotTable.data.

See documentation included in PR for more details.

closes #1076

@ambrosejcarr ambrosejcarr requested review from berl and shanaxel42 March 16, 2019 15:06
@codecov-io
Copy link

codecov-io commented Mar 16, 2019

Codecov Report

Merging #1087 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1087      +/-   ##
==========================================
+ Coverage    88.8%   88.89%   +0.08%     
==========================================
  Files         165      167       +2     
  Lines        6433     6481      +48     
==========================================
+ Hits         5713     5761      +48     
  Misses        720      720
Impacted Files Coverage Δ
starfish/types/_decoded_spots.py 100% <100%> (ø)
starfish/test/test_decoded_spots.py 100% <100%> (ø)
starfish/types/__init__.py 100% <100%> (ø) ⬆️
starfish/intensity_table/intensity_table.py 82.81% <100%> (+0.99%) ⬆️
starfish/types/_spot_attributes.py 84.21% <100%> (+3.25%) ⬆️
starfish/__init__.py 86.95% <0%> (-3.05%) ⬇️

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 dc138c8...5676896. Read the comment docs.

Copy link
Collaborator

@berl berl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good. Thanks!
Let's run with this and discuss with WG5 users about how to organize multi-cell assignment with probabilities within this framework. My suggestion would be further optional columns in pairs, i.e. cell_0, probability_0, cell_1, probability_1,... . And yes, we should really be able to stop at 10!

@ambrosejcarr
Copy link
Member Author

Let's run with this and discuss with WG5 users about how to organize multi-cell assignment with probabilities within this framework.

Sounds good. I'd also be fine adding a sparse matrix and saving both tables in an archive.

What would be your preferred way to proceed with WG5? CC then on this PR? email them? post this in slack?

@ambrosejcarr ambrosejcarr merged commit 9d5f897 into master Mar 19, 2019
Copy link
Collaborator

@ttung ttung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the case where this table does not have Z-planes? I'll let you decide. :)


Implementation
--------------
Starfish Implements the :py:class:`DecodedSpots` as a wrapper for the :code:`pd.DataFrame` object
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Starfish Implements the :py:class:`DecodedSpots` as a wrapper for the :code:`pd.DataFrame` object
Starfish implements the :py:class:`DecodedSpots` as a wrapper for the :code:`pd.DataFrame` object

spot coordinates (z, y, x) and gene target. Does not contain pixel coordinates.
"""
if Features.TARGET not in self.coords.keys():
raise RuntimeError(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise RuntimeError(
raise ValueError(


# test serialization
tempdir = tempfile.mkdtemp()
filename = os.path.join(tempdir, 'spots.csv')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably just use: with tempfile.NamedTemporaryFile(...)

required_fields = {
Coordinates.X.value, # spot x-coordinate
Coordinates.Y.value, # spot y-coordinate
Features.TARGET, # spot gene target
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some wonky alignment going on.

@ambrosejcarr ambrosejcarr deleted the ajc-decoded-spot-table branch March 22, 2019 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SpaceTx output format for decoded spots
4 participants