[spec] Specify object lifecycle and related operations #118

thetorpedodog · 2023-02-09T16:14:16Z

This formalizes the object lifecycle and the various creation, opening, and closing methods we are already using in the tiledbsoma Python implementation.

See #107.

This formalizes the object lifecycle and the various creation, opening, and closing methods we are already using in the tiledbsoma Python implementation.

bkmartinjr

Caveat: I did a very fast review. But based upon that, it looks good to me.

mlin

LGTM, one minor language tightening suggested inline

abstract_specification.md

Co-authored-by: Mike Lin <[email protected]>

johnkerl · 2023-02-10T14:44:40Z

abstract_specification.md

+
+SOMA objects are open for **exclusively** reading or writing.
+When a SOMA object is open for writing, only metadata, the schema, and information directly derived from the schema may be read.
+Additionally, for Collection objects, the members of the collection are also accessible when the collection is open in write mode.


say either Collection or collection but not Collection -- either it's a code reference or just common noun

johnkerl · 2023-02-10T14:47:08Z

abstract_specification.md

+writable_collection = soma_impl.open('file:///some/collection', 'w')
+dataframe = writable_collection['dataframe']  # OK
+dataframe.schema  # OK
+dataframe.read(...)  # This is NOT allowed and will cause an error.


I'd replace every 'This is NOT allowed' with more simply 'Not allowed' -- it's a little less shouty

I do get the idea of drawing attention to what they can't do -- but this entire section is about the allows/disallows. It needs no further highlighting.

For clarity, our internal specs tend to always use the RFC2119 standard (i.e., consistently use NOT), but given that this doc has not already adopted it, that is probably an aspirational change.

an author OUGHT TO use the caps consistently

@bkmartinjr TIL about RFC2119! :)

If we want to go that route, sure.

there are very good reasons for it - which is why the IETF uses it (as does many other orgs like W3C). It is a future decision which should not hold up this PR -- but coincidentally the topic was it was just raised inside CZI (regarding this very doc being unclear).

Here is an example of one of our specs that strongly benefits from it.

I filed #122 -- this is awesome info -- thanks @bkmartinjr !! :)

johnkerl · 2023-02-10T14:47:55Z

abstract_specification.md

+```
+
+```
+soma_collection_t soma_collection_create(char *uri, ...) { ... }


did you mean soma_collection_open here?

I was going for using a different function in each language. Using a different type will make this more obvious.

abstract_specification.md

johnkerl · 2023-02-10T14:49:17Z

abstract_specification.md

+writer.close()
+
+post_close = soma_impl.open('file:///write/here')
+post_close.read(...)  # WILL see the data written by write_data.


WILL -> will

johnkerl · 2023-02-10T14:56:27Z

abstract_specification.md

+For example:
+
+```
+root = soma_impl.open('path://netloc/root/collection', 'w')


I admire and applaud the explicit code bits -- speaking as an example of a non-super-abstract thinker (and, I claim, a good representative of a large proportion of the general reading public -- in contrast to the small number of amazing few people who are capable of thinking/thriving solely in the abstract, who often end up doing high-level design in their professional careers) I assert that this (pesudo)code is incredibly helpful.

That said, just to raise a flag for future work -- I know because this has bit me many times in the past -- these code-as-text bits don't auto-evolve as the actual code/spec evolve over time.

we should eventually move to an IDL-like definition if we want the spec to be durable. But as John says, that is future work, not immediately needed (IMHO)

abstract_specification.md

johnkerl · 2023-02-10T14:57:57Z

abstract_specification.md

+
+Parameters:
+
+- key: The key to add the new element at. This cannot already be a key of the Collection.


Collection -> collection

johnkerl · 2023-02-10T14:59:30Z

abstract_specification.md

+Parameters:
+
+- key: The key to add the new element at. This cannot already be a key of the Collection.
+- uri: An optional parameter to specify an exact URI to create the collection at. If the URI is relative, the new entry will be added as a relative URI. If the URI is absolute, the new entry will be added as an absolute URI. If not specified, the collection will generate a new URI for the entry based on a sanitized version of the key. When possible, this should be a relative URI based on the key. If a collection already exists at the user-provided URI, the operation should fail.


We need to say what relative URI does. (In particular, when we print URIs using the tiledb-py Group class, they print the same either way -- it's hard to "tell by looking".)

Namely:

Relative means if you recursively copy/move the storage, the parents will see the children in their new location.

Absolute means if you recursively copy/move the storage, the parents will see the children in their old location.

This must be stated explicitly.

Added discussion of relative and absolute URIs.

johnkerl · 2023-02-10T15:00:30Z

abstract_specification.md

-| write                                   | Write a subset of data to the SOMADataFrame                                    |
+| Operation                                | Description                                        |
+| ---------------------------------------- | -------------------------------------------------- |
+| static create(uri, ...) -> SOMADataFrame | Create a SOMADataFrame.                            |


ditto re end w/ period or not

thetorpedodog

lots of changes coming in hot

thetorpedodog · 2023-02-10T15:27:18Z

abstract_specification.md

+
+SOMA objects are open for **exclusively** reading or writing.
+When a SOMA object is open for writing, only metadata, the schema, and information directly derived from the schema may be read.
+Additionally, for Collection objects, the members of the collection are also accessible when the collection is open in write mode.


thetorpedodog · 2023-02-10T15:41:04Z

abstract_specification.md

+```
+
+```
+soma_collection_t soma_collection_create(char *uri, ...) { ... }


I was going for using a different function in each language. Using a different type will make this more obvious.

abstract_specification.md

thetorpedodog · 2023-02-10T15:44:25Z

abstract_specification.md

+writer.close()
+
+post_close = soma_impl.open('file:///write/here')
+post_close.read(...)  # WILL see the data written by write_data.


thetorpedodog · 2023-02-10T15:48:31Z

abstract_specification.md

+post_close.read(...)  # WILL see the data written by write_data.
+```
+
+Additionally, opened SOMA object instances are not guaranteed to see changes made by other writers after the open completed.


Went with a slightly different wording here.

abstract_specification.md

thetorpedodog · 2023-02-10T15:53:54Z

abstract_specification.md

+
+Parameters:
+
+- key: The key to add the new element at. This cannot already be a key of the Collection.


thetorpedodog · 2023-02-10T16:00:45Z

abstract_specification.md

+Parameters:
+
+- key: The key to add the new element at. This cannot already be a key of the Collection.
+- uri: An optional parameter to specify an exact URI to create the collection at. If the URI is relative, the new entry will be added as a relative URI. If the URI is absolute, the new entry will be added as an absolute URI. If not specified, the collection will generate a new URI for the entry based on a sanitized version of the key. When possible, this should be a relative URI based on the key. If a collection already exists at the user-provided URI, the operation should fail.


Added discussion of relative and absolute URIs.

thetorpedodog · 2023-02-10T16:02:32Z

abstract_specification.md

-Create a new SOMACollection with the user-specified URI.
+| Operation                               | Description                                                                              |
+| --------------------------------------- | ---------------------------------------------------------------------------------------- |
+| get(string key)                         | Get the object associated with the key                                                   |


Added for tables that have been changed.

thetorpedodog · 2023-02-10T16:25:40Z

abstract_specification.md

-| write                                   | Write a subset of data to the SOMADataFrame                                    |
+| Operation                                | Description                                        |
+| ---------------------------------------- | -------------------------------------------------- |
+| static create(uri, ...) -> SOMADataFrame | Create a SOMADataFrame.                            |


thetorpedodog · 2023-02-10T18:06:15Z

@bkmartinjr @mlin actually I would appreciate if you all took another look particularly for the description of URI semantics that I added.

@johnkerl I clicked you by accident but if you want to give it another ✅ be my guest

🆕 stuff about URIs and enums

abstract_specification.md

bkmartinjr

One suggestion for a clarification around relative URI resolution in collections. Otherwise LGTM

thetorpedodog · 2023-02-13T16:08:52Z

@atolopko-czi @aaronwolen since this is a spec change we do want unanimous approvals on this so please take a look. Thanks!

johnkerl · 2023-02-13T17:59:47Z

@thetorpedodog ✅ ✅

aaronwolen

This is really great

atolopko-czi

Really great to read the official details on open/close, read/write, etc.! A number of very minor questions and suggestions. Approving as is, since I found no blockers, but have a look at my comments before merging.

atolopko-czi · 2023-02-14T14:29:54Z

abstract_specification.md

+    - `.collection-info`
+      - `absolute`: `file:///soma-dataset/collection/abspath`
+      - `external-absolute`: `file:///soma-dataset/more-data/other-data`
+      - `relative`: `relpath`


[minor] This is a little confusing to me, since this example seems to be mixing a representation of file system directory layout with collection contents, if I understand correctly. It's understandable, but could possibly separate these two representations for clarity. "If we have this collection: ... it will be stored in the filesystem as follows: ..."

Updated to represent .collection-info as a real file and present its contents differently from other paths.

abstract_specification.md

atolopko-czi · 2023-02-14T14:37:41Z

abstract_specification.md

+
+| Operation                      | Description                                                                                                  |
+| ------------------------------ | ------------------------------------------------------------------------------------------------------------ |
+| static open(string uri, ...)   | Opens the SOMA object (of the type it is attached to).                                                       |


Should (or does) the spec define the behavior for attempting to open an object that does not exist or cannot be read? I suppose that's implementation-specific, such as whether an exception should be thrown or nil/null/none is returned. But it would seem reasonable to specify at the spec level that a SOMAObject will not instantiated at all in such cases. Rather than getting an object that can then be inspected for its validity.

And I see the create() operation does mention the error case behavior, below.

Added a note in the section below.

atolopko-czi · 2023-02-14T14:46:13Z

abstract_specification.md

+
+The `create` static method **creates** a new stored SOMA object, then returns that object opened in write mode.
+
+If an object already exists at the given location, this should throw an error.


Since data is not necessarily flushed until close, do we need to specify semantics for multiple processes attempting to create an object at the same URI in parallel? Is the "write lock" guaranteed upon the return of create() or close()?

Added a note that the creation operation should be atomic.

atolopko-czi · 2023-02-14T14:54:11Z

abstract_specification.md

+This completes all writes (if applicable) and releases other resources associated with this SOMA object.
+Implementations of `close` must be idempotent.
+
+If the implementation permits it, a write operation should function atomically; i.e. any other reader should not see partially-completed reads _until_ the close operation is called.


Could be its own section, since it involves both open and close. And I see open doesn't mention atomicity semantics. Both open and close could have "see atomicity semantics section, below", e.g.

I think after this it is certainly worth looking at reorganizing the way we discuss atomicity and read/write semantics; what we have is pretty complete but is scattered across multiple places.

atolopko-czi · 2023-02-14T15:10:00Z

abstract_specification.md

+Parameters:
+
+- key: The key to add the new element at. This cannot already be a key of the collection.
+- uri: An optional parameter to specify a URI to create the new collection, which may be [relative or absolute](#collection-entry-uris). If the URI is relative, the new entry will be added with that relative URI. If the URI is absolute, the new entry will be added with that absolute URI. If a collection already exists at the user-provided URI, the operation should fail. If the user does not specify a URI, the collection will generate a new URI for the entry. When possible, this should be a relative URI based on a sanitized version of the key.


Do this need to mention that relative URIs cannot be "multi-level"? I think that's what the spec mentions above.

My feeling is that this is covered by the linked reference to how relative URIs work, and that adding something specifically here would make this already-long paragraph even longer.

atolopko-czi · 2023-02-14T15:12:12Z

abstract_specification.md

+`add_new_collection` has an extra parameter allowing control over the type of collection added:
+
+```
+add_new_collection(string key, CollectionType type, string uri = "", PlatformConfig platform_config) -> SOMACollection


Completely out-of-scope for this PR, but I'm now just wondering why we didn't consider having explicit methods for the various types of collections? add_new_experiment, add_new_measurement, add_new_collection.

Guessing what my motives were back then: all collections work the same in terms of adding, so this avoided having three very similar methods. It’s not unreasonable to have one for each collection type, though.

atolopko-czi · 2023-02-14T15:17:57Z

abstract_specification.md

+| get schema -> Arrow.Schema               | Return data schema, in the form of an Arrow Schema. |
+| get index_column_names -> [string, ...]  | Return index (dimension) column names.              |
+| get count -> int                         | Return the number of rows in the SOMADataFrame.     |
+| read                                     | Read a subset of data from the SOMADataFrame.       |


Read a subset of data iteratively from the SOMADataFrame? Whether or not one is reading a subset, the iterative nature of reads seems like a core part of the design, even at the spec level, given the returns type for DataFrame.read (and SparseNDArray.read())

While most people bothering to read a spec like this will understand that a subset includes the whole set, do we want to rely upon reader knowing that? "Read all or a subset..."

I’m inclined to think we’re OK here. If this deserves clarification anywhere, it would be in the section that describes the read and write methods.

atolopko-czi · 2023-02-14T15:24:29Z

abstract_specification.md

+| get is_sparse -> True                        | Return the constant True.                                               |
+| get nnz -> uint                              | Return the number stored values in the array, including explicit zeros. |
+| read                                         | Read a slice of data from the SOMASparseNDArray.                        |
+| write                                        | Write a slice of data to the SOMASparseNDArray.                         |


important to use slice here instead of subset as we do above? If it's a technical term we explicitly want, ok.

This is a reasonable question, but since this is a whitespace-change–only line, I think this is better handled in a followup.

atolopko-czi · 2023-02-14T15:27:59Z

abstract_specification.md

@@ -608,6 +948,7 @@ Array read operations can return results in a variety of formats. The `SOMABatch
 Summary:

 ```
+open(string uri, ...) -> SOMA object      # identify a SOMA object and open it


Suggested change

open(string uri, ...) -> SOMA object # identify a SOMA object and open it

open(string uri, ...) -> SOMAObject # identify a SOMA object and open it

?

What I’m trying to imply here (maybe unsuccessfully) is that the thing you get from open is some SOMA object, and not necessarily an object whose exact type is SOMAObject.

Co-authored-by: Andrew Tolopko <[email protected]>

thetorpedodog

This is it! Thank you everybody for the great feedback; it has helped a lot.

thetorpedodog · 2023-02-14T20:38:31Z

abstract_specification.md

@@ -608,6 +948,7 @@ Array read operations can return results in a variety of formats. The `SOMABatch
 Summary:

 ```
+open(string uri, ...) -> SOMA object      # identify a SOMA object and open it


What I’m trying to imply here (maybe unsuccessfully) is that the thing you get from open is some SOMA object, and not necessarily an object whose exact type is SOMAObject.

thetorpedodog · 2023-02-14T20:40:03Z

abstract_specification.md

+| get is_sparse -> True                        | Return the constant True.                                               |
+| get nnz -> uint                              | Return the number stored values in the array, including explicit zeros. |
+| read                                         | Read a slice of data from the SOMASparseNDArray.                        |
+| write                                        | Write a slice of data to the SOMASparseNDArray.                         |


This is a reasonable question, but since this is a whitespace-change–only line, I think this is better handled in a followup.

thetorpedodog · 2023-02-14T20:41:26Z

abstract_specification.md

+| get schema -> Arrow.Schema               | Return data schema, in the form of an Arrow Schema. |
+| get index_column_names -> [string, ...]  | Return index (dimension) column names.              |
+| get count -> int                         | Return the number of rows in the SOMADataFrame.     |
+| read                                     | Read a subset of data from the SOMADataFrame.       |


I’m inclined to think we’re OK here. If this deserves clarification anywhere, it would be in the section that describes the read and write methods.

thetorpedodog · 2023-02-14T20:44:59Z

abstract_specification.md

+`add_new_collection` has an extra parameter allowing control over the type of collection added:
+
+```
+add_new_collection(string key, CollectionType type, string uri = "", PlatformConfig platform_config) -> SOMACollection


Guessing what my motives were back then: all collections work the same in terms of adding, so this avoided having three very similar methods. It’s not unreasonable to have one for each collection type, though.

thetorpedodog · 2023-02-14T20:47:02Z

abstract_specification.md

+Parameters:
+
+- key: The key to add the new element at. This cannot already be a key of the collection.
+- uri: An optional parameter to specify a URI to create the new collection, which may be [relative or absolute](#collection-entry-uris). If the URI is relative, the new entry will be added with that relative URI. If the URI is absolute, the new entry will be added with that absolute URI. If a collection already exists at the user-provided URI, the operation should fail. If the user does not specify a URI, the collection will generate a new URI for the entry. When possible, this should be a relative URI based on a sanitized version of the key.


My feeling is that this is covered by the linked reference to how relative URIs work, and that adding something specifically here would make this already-long paragraph even longer.

thetorpedodog · 2023-02-14T20:50:24Z

abstract_specification.md

+
+| Operation                      | Description                                                                                                  |
+| ------------------------------ | ------------------------------------------------------------------------------------------------------------ |
+| static open(string uri, ...)   | Opens the SOMA object (of the type it is attached to).                                                       |


Added a note in the section below.

thetorpedodog · 2023-02-14T20:50:46Z

abstract_specification.md

+
+The `create` static method **creates** a new stored SOMA object, then returns that object opened in write mode.
+
+If an object already exists at the given location, this should throw an error.


Added a note that the creation operation should be atomic.

thetorpedodog · 2023-02-14T20:52:20Z

abstract_specification.md

+This completes all writes (if applicable) and releases other resources associated with this SOMA object.
+Implementations of `close` must be idempotent.
+
+If the implementation permits it, a write operation should function atomically; i.e. any other reader should not see partially-completed reads _until_ the close operation is called.


I think after this it is certainly worth looking at reorganizing the way we discuss atomicity and read/write semantics; what we have is pretty complete but is scattered across multiple places.

thetorpedodog · 2023-02-14T20:54:15Z

abstract_specification.md

+
+A SOMA collection also manages the lifecycle of objects directly instantiated by it.
+Objects accessed via getting a Collection element, or objects created with one of the <code>add_new\_<var>object_type</var></code> methods are considered "owned" by the collection.
+All such objects will be automatically closed together by [the collection's close operation](#operation-close-collection-types).


This is mentioned in the detailed description of add_new_whatever below.

thetorpedodog · 2023-02-14T21:10:30Z

abstract_specification.md

+    - `.collection-info`
+      - `absolute`: `file:///soma-dataset/collection/abspath`
+      - `external-absolute`: `file:///soma-dataset/more-data/other-data`
+      - `relative`: `relpath`


Updated to represent .collection-info as a real file and present its contents differently from other paths.

Specify object lifecycle and related operations.

ed93822

This formalizes the object lifecycle and the various creation, opening, and closing methods we are already using in the tiledbsoma Python implementation.

thetorpedodog requested review from mlin, aaronwolen, johnkerl, bkmartinjr and atolopko-czi February 9, 2023 16:14

bkmartinjr previously approved these changes Feb 9, 2023

View reviewed changes

mlin previously approved these changes Feb 9, 2023

View reviewed changes

abstract_specification.md Outdated Show resolved Hide resolved

Tighten language around collection closing

0bb0868

Co-authored-by: Mike Lin <[email protected]>

johnkerl changed the title ~~Specify object lifecycle and related operations.~~ [python] Specify object lifecycle and related operations Feb 10, 2023

johnkerl requested changes Feb 10, 2023

View reviewed changes

johnkerl mentioned this pull request Feb 10, 2023

Be consistent with backticks et al. in the SOMA spec #120

Closed

thetorpedodog changed the title ~~[python] Specify object lifecycle and related operations~~ [spec] Specify object lifecycle and related operations. Feb 10, 2023

johnkerl mentioned this pull request Feb 10, 2023

Consider RFC2119 conventions for the SOMA spec #122

Open

thetorpedodog added 3 commits February 10, 2023 11:28

Cleanup and rephrasing.

9645459

Add enum types and description of URI semantics.

cac1ac0

rm batch_size

9310aba

thetorpedodog commented Feb 10, 2023

View reviewed changes

thetorpedodog requested a review from johnkerl February 10, 2023 17:44

johnkerl approved these changes Feb 10, 2023

View reviewed changes

thetorpedodog requested review from johnkerl, bkmartinjr and mlin February 10, 2023 18:04

bkmartinjr reviewed Feb 10, 2023

View reviewed changes

abstract_specification.md Show resolved Hide resolved

bkmartinjr approved these changes Feb 10, 2023

View reviewed changes

Add notes on hierarchies.

0706697

johnkerl changed the title ~~[spec] Specify object lifecycle and related operations.~~ [spec] Specify object lifecycle and related operations Feb 10, 2023

thetorpedodog force-pushed the object-lifecycle branch from 096f546 to 0706697 Compare February 10, 2023 22:13

johnkerl approved these changes Feb 10, 2023

View reviewed changes

aaronwolen approved these changes Feb 13, 2023

View reviewed changes

atolopko-czi approved these changes Feb 14, 2023

View reviewed changes

mlin approved these changes Feb 14, 2023

View reviewed changes

thetorpedodog and others added 2 commits February 14, 2023 15:54

Read/write and add_new clarifications

f93d58c

Co-authored-by: Andrew Tolopko <[email protected]>

Clarify URI filesystem hierarchy, operation atomicity.

426b66e

thetorpedodog commented Feb 14, 2023

View reviewed changes

thetorpedodog merged commit 582f12e into main Feb 14, 2023

thetorpedodog deleted the object-lifecycle branch February 14, 2023 21:12


		Parameters:

		- key: The key to add the new element at. This cannot already be a key of the Collection.


		The `create` static method creates a new stored SOMA object, then returns that object opened in write mode.

		If an object already exists at the given location, this should throw an error.

	open(string uri, ...) -> SOMA object # identify a SOMA object and open it
	open(string uri, ...) -> SOMAObject # identify a SOMA object and open it

[spec] Specify object lifecycle and related operations #118

[spec] Specify object lifecycle and related operations #118

Conversation

thetorpedodog commented Feb 9, 2023

bkmartinjr left a comment

Choose a reason for hiding this comment

mlin 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

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

johnkerl Feb 10, 2023 • edited Loading

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

thetorpedodog 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

thetorpedodog commented Feb 10, 2023

bkmartinjr left a comment

Choose a reason for hiding this comment

thetorpedodog commented Feb 13, 2023

johnkerl commented Feb 13, 2023

aaronwolen left a comment

Choose a reason for hiding this comment

atolopko-czi 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thetorpedodog 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnkerl Feb 10, 2023 •

edited

Loading