Add the Scene class
#220
Merged
Add the Scene class
#220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jp-dark
requested review from
aaronwolen,
johnkerl,
bkmartinjr and
ivirshup
bkmartinjr
approved these changes
Sep 16, 2024
- Add new MultiscaleImage - Add new SpatialDataframe abstract base class - Add PointCloud subclass of the SpatialDataFrame - Add GeometryDataFrame subclass of the SpatialDataFrame - Add shapely as a dependency
* Update docstrings * Add ValueError error messages in `SpatialRead` * Rename method `read_level`->`read_region`
jp-dark
force-pushed
the
dark/spatial-datatypes
branch
from
34f7878
to
462678a
Compare
Co-authored-by: nguyenv <vivian@tiledb.com>
johnkerl
approved these changes
Sep 19, 2024
The individual implementations may find a shared base class useful, but it doesn't need to be included in the abstract specification.
* Create the `Scene` class * Add `spatial` collection of scenes to the `Experiment` class * Add Scene to ephemeral collections and tests
|
[sc-55577], [sc-55578], [sc-55579] |
ivirshup
reviewed
Sep 23, 2024
python-spec/src/somacore/scene.py
Outdated
Comment on lines
115
to
122
| If the subcollection the geometry dataframe is inside of is more than one | ||
| layer deep, the input should be provided as a sequence of names. For example, | ||
| to register a geometry dataframe named "transcripts" in the "var/RNA" | ||
| collection:: | ||
|
|
||
| scene.register_geometry_dataframe( | ||
| 'transcripts', transform, subcollection=['var', 'RNA'], | ||
| ) |
There was a problem hiding this comment.
Is this a workflow that is being supported? I thought the spec was pretty opinionated about where each kind of thing would be stored.
There was a problem hiding this comment.
The example in the docstring follows the storage guideline and is more than one layer deep.
aaronwolen
approved these changes
Sep 25, 2024
python-spec/src/somacore/scene.py
Outdated
| raise NotImplementedError() | ||
|
|
||
| @abc.abstractmethod | ||
| def get_transformation_to_multiscale_image( |
There was a problem hiding this comment.
Suggested change
| def get_transformation_to_multiscale_image( | |
| def get_transformation_multiscale_image( |
Do we need to here?
There was a problem hiding this comment.
We need something to specify what the transform is going from/to. (Scene to image is different than image to scene). I don't love how verbose the methods are, so I'm open to other suggestions. I did change transformation to match transform (based on one of your suggestion changes on #219).
* Add methods to add new assets to a scene * Rename `register_*` methods to `set_transformation_to_*`
* Rename `_transformation_` methods to `_transform_`. * Add ability to get transforms "from" as well as "to" assets.
An error occurred while trying to automatically change base from
dark/spatial-datatypes
to
main
September 26, 2024 16:28
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sceneclassspatialcollection of scenes to theExperimentclassThis PR is ready for review, but should be merged after PR #219