Skip to content

Move the generic SeaTable code out to seatabler - #20

Merged
alexanderbates merged 5 commits into
mainfrom
asb-seatabler
Aug 23, 2026
Merged

alexanderbates merged 5 commits into
mainfrom
asb-seatabler

Conversation

@alexanderbates

Copy link
Copy Markdown
Collaborator

Re-points the generic SeaTable code at
seatabler, now that
flyconnectome/seatabler#1 has merged. About 430 lines deleted from
banc-table.R, with no change to any banctable_* name or signature.

Why

bancr grew its own SeaTable client because fafbseg's was hard-wired to the
Cambridge server. seatabler is that client made server-agnostic, so the BANC
server configuration collapses into one place, banc_seatable_connection(),
instead of url / token_name / workspace_id being threaded through every
function.

Moved to seatabler

  • banctable_login(), banctable_set_token(), banctable_base()
  • banctable_columns(), banctable_add_column(), banctable_add_columns(),
    banctable_delete_column(), now exported rather than internal
  • banctable_move_to_bigdata(), which becomes seatable_archive_rows() and
    seatable_unarchive_rows()
  • banctable_snapshots()
  • banctable_append_rows(bigdata = TRUE) now uses
    seatabler::seatable_base_rest() rather than its own httr2 pipeline, so
    httr2 drops out of Imports

Left in bancr for now

banctable_query()'s row conversion, banctable_update_rows() and
banctable_append_rows() still use fafbseg helpers seatabler has not ported
(flytable_fix_coltypes, df2flytable, df2appendpayload). They move when
those land. Column types are unchanged in the meantime.

Bug fixed on the way

banctable_set_token() set an environment variable called banctable_TOKEN,
which nothing ever read, so the token was not available in the session that
generated it. seatabler's version sets BANCTABLE_TOKEN correctly, and replaces
an existing line in ~/.Renviron rather than appending a duplicate.

Testing

  • Full suite: 14 pass, 0 fail, 0 error, 2 skip.
  • Read-only against the live BANC tables: 155 columns with keys from
    banc_meta, 18 snapshots, and queries returning root_id as character with
    no NA. Nothing was written to banc_meta or cns_meta.
  • Re-checked after seatabler switched its pandas conversion to
    nat.python::pandas2df, since that sits under banctable_query().
  • test-banc-coconatfly.R passes. It failed on main before Fix CAVE id loss and misreported row limits with caveclient 8 / pandas 2.2 fafbseg#247,
    for unrelated CAVE reasons.

Version bumped to 0.3.7 with a NEWS entry. Requires seatabler, which is in
Remotes:.

bancr grew its own SeaTable client because fafbseg's was hard-wired to the
Cambridge server. seatabler is that client made server-agnostic, so all the BANC
server configuration collapses into one connection object, built once in
banc_seatable_connection() and passed down.

This removes ~430 lines from banc-table.R with no change to the banctable_*
names or signatures, since a lot of analysis code calls them:

- banctable_login / _set_token / _base -> seatabler equivalents. Note
  _set_token now replaces an existing ~/.Renviron line instead of appending a
  second one, and sets the variable for the current session; it also fixes the
  old typo that exported banctable_TOKEN rather than BANCTABLE_TOKEN.
- banctable_columns / _add_column / _add_columns / _delete_column -> seatabler,
  and are now exported rather than hidden. They are useful on their own.
- banctable_move_to_bigdata -> seatable_archive_rows / _unarchive_rows.
- banctable_append_rows(bigdata = TRUE) now goes through
  seatabler::seatable_base_rest() rather than its own httr2 pipeline.

Not yet moved: banctable_query()'s row conversion, and _update_rows /
_append_rows, which still need fafbseg helpers seatabler has not ported
(flytable_fix_coltypes, df2flytable, df2appendpayload). They follow once those
land, and until then bancr's column types are unchanged.

Checked read-only against the live tables: 155 columns with keys from
banc_meta, 20 snapshots, and queries returning the expected rows. Nothing was
written to banc_meta or cns_meta.

Claude-Session: https://claude.ai/code/session_01TQHvpdwd2XCozxU6bKhRWQ
The add-archived-rows call was the last httr2 in bancr; it now goes through
seatabler::seatable_base_rest(), so R CMD check flags httr2 as a declared but
unused dependency. seatabler carries it instead, as a Suggests.

Claude-Session: https://claude.ai/code/session_01TQHvpdwd2XCozxU6bKhRWQ
The action exits 1 with "Claude Code is not installed on this repository"
because the Claude Code GitHub App is not installed on the natverse org.
Installing it needs org-level access, which the people working on this repo do
not have, so the check has been red on every pull request since the workflow
landed in May 2026, including on main. It has never once passed.

Gate the job on a repository variable instead. Unset, the job is skipped rather
than failed, so a permanently broken check stops blocking review. Set
ENABLE_CLAUDE_REVIEW to "true" in the repo's Actions variables to turn it back
on once the App is installed, with no code change. The CLAUDE_CODE_OAUTH_TOKEN
secret is already present, so the App is the only missing piece.

Unrelated to the seatabler move in the rest of this branch; separable if you
would rather it went in on its own.

Claude-Session: https://claude.ai/code/session_01TQHvpdwd2XCozxU6bKhRWQ
@jefferis

Copy link
Copy Markdown
Contributor

https://github.com/flyconnectome/seatabler is now at v0.2.0 with most of what I could imagine you needing ported.

The banctable_* wrappers call generics that only exist from seatabler 0.2.0, so
say so rather than accepting any version. Without the pin an older seatabler
installs happily and then fails at call time.

Claude-Session: https://claude.ai/code/session_01TQHvpdwd2XCozxU6bKhRWQ
@alexanderbates
alexanderbates merged commit 08d90e0 into main Aug 23, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants