Update python package and builder to tiledbsoma 1.0.0rc2 #227
Conversation
bkmartinjr
changed the title
Codecov Report
@@ Coverage Diff @@
## main #227 +/- ##
==========================================
- Coverage 84.02% 83.75% -0.28%
==========================================
Files 29 29
Lines 1628 1619 -9
==========================================
- Hits 1368 1356 -12
- Misses 260 263 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
tools/cell_census_builder/globals.py
Outdated
|
|
||
| # Using "end of time" for read_timestamp means that all writes are visible, no matter what write timestamp was used | ||
| END_OF_TIME = 0xFFFFFFFFFFFFFFFF | ||
|
|
||
|
|
||
| def SOMA_TileDB_Context() -> soma.options.SOMATileDBContext: | ||
| global _SOMA_TileDB_Context | ||
| if _SOMA_TileDB_Context is None or _SOMA_TileDB_Context != TileDB_Ctx(): |
There was a problem hiding this comment.
To fix a bug I introduced on an earlier PR:
| if _SOMA_TileDB_Context is None or _SOMA_TileDB_Context != TileDB_Ctx(): | |
| if _SOMA_TileDB_Context is None or _SOMA_TileDB_Context.tiledb_ctx != TileDB_Ctx(): |
There was a problem hiding this comment.
But maybe it's time to just eliminate support for custom tiledb_ctx values (remove the setter), since we don't use it, allowing us to remove this check.
There was a problem hiding this comment.
Yeah, for now, I'll just make it a cached value. Good call.
tools/cell_census_builder/globals.py
Outdated
| read_timestamp=END_OF_TIME, | ||
| # `None` opts out of explicit timestamp consistency, and uses the underlying | ||
| # TileDB per-object write consistency. This is sufficient given that this | ||
| # builder explicitly orders / synchronizes all reading/writing. |
There was a problem hiding this comment.
As "timestamp consistency [is] the default" (per this comment), we could remove this comment. Minimally, update it to be accurate or reword so that it doesn't become outdated if/when TileDB-SOMA semantics change again.
There was a problem hiding this comment.
you are right that this is confusing. I meant not using PIT consistency across the entire census collection. I'll delete.
tools/cell_census_builder/globals.py
Outdated
| ) | ||
| return _SOMA_TileDB_Context | ||
|
|
||
|
|
||
| def TileDB_Ctx() -> tiledb.Ctx: | ||
| # See `DEFAULT_TILEDB_CONFIG` above |
There was a problem hiding this comment.
It could be set to something other than DEFAULT_TILEDB_CONFIG. But if we're never going to do that in builder code, I suppose we could remove the ability to set it altogether, in which case this comment is fine.
There was a problem hiding this comment.
resolved by deleting setter
Update cell census package and builder to utilize
tiledbsoma==1.0.0rc2Other changes:
Fixes #208
Fixes #221
Fixes #228