Complete the seatabler migration: queries and row writes - #21
Open
alexanderbates wants to merge 2 commits into
Open
Complete the seatabler migration: queries and row writes#21alexanderbates wants to merge 2 commits into
alexanderbates wants to merge 2 commits into
Conversation
seatabler 0.2.0 added seatable_update_rows() and seatable_append_rows(), the generics banctable_update_rows() / banctable_append_rows() were waiting on. Both now delegate, dropping another ~215 lines. Names, signatures and defaults are unchanged, including append_allowed = FALSE and bigdata = FALSE. Multiple-select columns are now detected from the table schema by seatabler rather than comma-split by hand here, which also fixes a latent bug: a multi-select value containing a comma used to be split into two options. banctable_query() deliberately keeps bancr's own row conversion. seatable_query() does not apply column types, so delegating it today would change 11 of 163 column classes on banc_meta: status and cell_type_source become list columns, _ctime/_mtime lose POSIXct, and two number columns arrive as character. Reported as flyconnectome/seatabler#9; the switch waits on that. Adds a test that reaches seatable_module() and banctable_login(). bancr's suite previously passed even with a seatabler that could not reach its Python dependencies, because nothing exercised that path -- a stale nat.python broke every SeaTable call while the tests stayed green. Round-tripped against a throwaway base on cloud.seatable.io: append three rows, update them, delete them, back to the starting row count with no leftovers. Nothing was written to banc_meta or cns_meta. Full suite 23 pass, 0 fail. Claude-Session: https://claude.ai/code/session_01TQHvpdwd2XCozxU6bKhRWQ
seatabler 0.2.2 applies column types from the table schema (st_coerce_df), which was the one thing keeping banctable_query()'s row conversion here. It now delegates like the rest, and banctable2df() retires with it. That removes the last fafbseg::: internal from banc-table.R. Reads are unchanged, checked against a snapshot taken before the switch on 500 rows of banc_meta: same 163 columns in the same order, and identical values for root_id, status and cell_type across every row. status stays character, so banc_update_status() and the string handling around it are unaffected. Five number columns (input_connections, output_connections, mitochondria, mitochondria_volume, cell_ids_id) come back as integer rather than numeric, because st_fix_coltypes() compares mode() and mode(integer) is "numeric". No effect on values: the largest across 3000 rows is 3.8 million against a 2^31 limit, and anything larger would arrive from pandas as a double anyway. Noted on flyconnectome/seatabler#9 rather than worked around here. limit = FALSE is still honoured, mapped to an infinite limit. `ac` stays in the signature and is ignored, as with the other wrappers. Requires seatabler (>= 0.2.2). Full suite 23 pass, 0 fail. Claude-Session: https://claude.ai/code/session_01TQHvpdwd2XCozxU6bKhRWQ
alexanderbates
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Follow-up to #20, completing the migration using seatabler 0.2.0's write
generics and 0.2.2's column coercion. Around 375 lines out of
banc-table.R,with no change to any
banctable_*name, signature or default.Moved across
banctable_update_rows()->seatabler::seatable_update_rows()banctable_append_rows()->seatabler::seatable_append_rows(), whichhandles the big-data path itself, so
bigdata = TRUEno longer needs bancr'sown REST call
banctable_query()->seatabler::seatable_query()banc_df2updatepayload()andbanctable2df()retiredThat removes the last
fafbseg:::internal frombanc-table.R. Defaults arepreserved, including
append_allowed = FALSE(seatabler's own default isTRUE),bigdata = FALSE, andlimit = FALSEmeaning "no LIMIT clause".Reads are unchanged
banctable_query()was the piece held back, becauseseatable_query()did notapply column types until seatabler 0.2.2 (flyconnectome/seatabler#9, now fixed
and confirmed). Checked against a snapshot taken before the switch, 500 rows of
banc_meta:root_id,statusandcell_typeon every rowstatusstays character, sobanc_update_status()and the string handlingaround it are unaffected
Five
numbercolumns (input_connections,output_connections,mitochondria,mitochondria_volume,cell_ids_id) now come back asintegerrather than
numeric, becausest_fix_coltypes()comparesmode()andmode(integer)is"numeric". No effect on values: across 3000 rows thelargest is 3,773,658 against a 2^31 limit, and anything larger would arrive from
pandas as a double rather than overflowing. Recorded on seatabler#9 rather than
worked around here.
Fixes a latent bug
Multiple-select columns were comma-split by hand, so a value that itself
contained a comma was silently broken into two options. seatabler detects
multi-select from the table schema and passes whole values.
Test coverage gap closed
New
test-seatabler.Rreachesseatable_module()andbanctable_login(),skipping without a token. Worth having: bancr's suite passed cleanly while a
stale nat.python made every SeaTable call fail, because nothing exercised that
path. That is also why #20 needed a
seatablerversion pin.Testing
rows, update them, verify the new values, delete them, back to the starting
row count with no leftovers.
banc_metaorcns_metaat any point; the read checksabove are read-only.
Requires seatabler (>= 0.2.2). Version 0.3.8.