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

[python] Enforce if-not-exists semantics for append/registration #2384

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Apr 4, 2024

Issue and/or context:

There are two use-cases for registration:

  • The SOMA Experiment does not exist, and there are one or more AnnData objects to be registered and then ingested.
    • This user intent is signaled by passing experiment_uri=None to the registrar
  • The SOMA Experiment does exist, and there are one or more AnnData objects to be co-registered with the existing experiment, and then the AnnData objects are to be ingested
    • This user intent is signaled by passing a non-empty experiment_uri to the registrar

Due to an unforunate implementation bug (in the code touched by this PR) these two use-cases were getting conflated.

Changes:

  • Raise an exception if the experiment_uri passed to the registrar is not None. This is a non-breaking change, since the existing docstrings already make it sound like what this PR is doing was already the case.
  • Make the docstrings even clearer
  • Unit-test the behavior

Notes for Reviewer:

[sc-44602]

Copy link

This pull request has been linked to Shortcut Story #44602: AnnData registration silently fails when unauthenticated.

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Merging #2384 (9286b1e) into main (f0c483d) will increase coverage by 24.55%.
The diff coverage is 100.00%.

❗ Current head 9286b1e differs from pull request most recent head dd67eca. Consider uploading reports for the commit dd67eca to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2384       +/-   ##
===========================================
+ Coverage   66.04%   90.60%   +24.55%     
===========================================
  Files         144       37      -107     
  Lines       13005     3895     -9110     
  Branches      511        0      -511     
===========================================
- Hits         8589     3529     -5060     
+ Misses       4316      366     -3950     
+ Partials      100        0      -100     
Flag Coverage Δ
libtiledbsoma ?
python 90.60% <100.00%> (-0.01%) ⬇️
r ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
python_api 90.60% <100.00%> (-0.01%) ⬇️
libtiledbsoma ∅ <ø> (∅)

@johnkerl johnkerl merged commit c6a6bcc into main Apr 4, 2024
9 checks passed
@johnkerl johnkerl deleted the kerl/append-uri-existence-check branch April 4, 2024 22:47
johnkerl added a commit that referenced this pull request Apr 4, 2024
…) (#2387)

Co-authored-by: John Kerl <kerl.john.r@gmail.com>
@johnkerl
Copy link
Member Author

johnkerl commented Apr 5, 2024

[sc-44675]

Copy link

This pull request has been linked to Shortcut Story #44675: tiledbsoma 1.9.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants