Let a table declare its dimensions, and a directory of data carry its sets

[d-chambers]

Description

Phase 3b of the annotation roadmap, second half, and the last of the store conventions. Stacked on #930 — review that one first; this PR's own diff is the second commit.

Two things a stored set could not do yet.

A table may declare its own dimensions. A bare table has no attrs file to state them in, so the call had to, and a picker handing over picks.csv had to hand over its dimensions separately. It may now say so above its header:

# dims: distance, time
group,time_start,time_end

A comment, deliberately: a reader which knows nothing of this convention still reads the table, which column-name markup or a second header row would break. Restating the dimensions is allowed where the spellings agree and refused where they differ, as everywhere else in this format — there is no precedence rule between two spellings of one fact. Vertices declare nothing: they are read in the dimensions of the set they belong to, which states them once. Comment lines above either table are otherwise just comments.

They are read, not written. to_csv keeps writing a plain table, because one every reader can open is worth more here than a self-describing one; the pragma is the hand-authoring spelling, and the docstrings now say so.

read_table grew a skip argument for this — lines above the header — and row numbers in its errors still count from the top of the file, so they name the line a reader would look at.

A directory of data carries what it was annotated with. Hidden, under .annotations, exactly as it carries its inventory under .inventory: the directory .annotations/ holds a set or a directory of sets, and .annotations.csv is the bare-table spelling. Hidden so the file scanner does not read it as data, and so a directory holding a visible annotations.csv stays a set rather than something carrying one — the discovery only fires where a directory states nothing itself. Two spellings at once, or something under the name in a form that name does not take, are refused rather than guessed at, as find_inventory refuses them. find_annotations is the sibling of find_inventory; there is no carries_annotations until a spool asks the cheap question.

So dc.annotations(path) now takes the same path dc.spool(path) does.

Still outstanding in 3b: the optional Parquet encoding. It wants a decision rather than a default — nothing in DASCore reads or writes Parquet today and pyarrow is in no test environment, so it would land untested unless it is added as a test dependency. Happy to do it either way.

Changelog

• added: An annotation table may declare its dimensions in a # dims: distance, time line above its header.
• added: A directory of data carries the annotations made on it under .annotations, and dc.annotations reads it from the data directory.
• added: dascore.utils.tables.read_table takes a skip argument for lines above the header.

Checklist

I have:

• filled in the Changelog section above (see docs/contributing/general_guidelines.qmd).

I have (if applicable):

• referenced the GitHub issue this PR closes.
• documented the new feature with docstrings and/or appropriate doc page.
• included tests. See testing guidelines.
• added the "ready_for_review" tag once the PR is ready to be reviewed.

Review

Five-lens adversarial pass (Codex is over its usage limit until Aug 20, so Claude-only). 33 findings; answered in the second commit:

• A header may legitimately start with #. A column named #note had its header eaten as a comment and the first row of data promoted to header — silently, and a regression against the previous PR where nothing was ever skipped. Found by the correctness and blast-radius reviewers independently. A comment is now the mark and a space, a doubled mark, or the pragma itself.
• skip counted csv records where pandas counted lines, so a quote inside a skipped comment could hand pandas a different header than the one this checked. read_table now opens once and lets pandas read on from the same stream; skiprows is gone.
• A child declaring its dimensions above its table was invisible to the collection loader, so the same layout worked with an attrs.json and failed with a pragma. And a message told the caller their dims "was given" when the dims came from the directory beside the sets.
• find_annotations raised ParameterError where its documented sibling find_inventory raises the inventory's own error, and where every other failure of dc.annotations is an InvalidAnnotationError.
• An unreadable table advised writing the very line it already holds — the pragma scan swallowed the decode error. It now says what it could not read.
• Prose: three claims were disproved by running them, including "a reader which knows nothing of this convention still reads the table" (pd.read_csv takes the pragma for the header) and the dims parameter's promise that a caller may override what a source states.
• Tests: four pragma tests asserted only that .dims echoed the pragma back — dropping the skip entirely left them green while the loaded table's only column was the comment. They now assert the table parsed. Added: a #-header regression test, a struck-out ## dims:, case-insensitivity, a blank line above the declaration, a comment holding an unbalanced quote, an unreadable table, and a data directory whose own attrs.json must not block discovery.

Not taken: find_annotations still duplicates find_inventory's shape rather than sharing a helper — the two differ in error type and wording, and unifying them would edit the inventory's tested messages for no behavior change.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (3)

• dev
• master
• main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0c12751e-5c5c-44ff-811b-326770e35c43

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[d-chambers]

Codex review (cross-model leg, now that credits are back). One finding, addressed:

• The door was inconsistent about attrs/vertices. Every directory refused them before discovering whether it merely carries a bare .annotations.csv, so dc.annotations(data / ".annotations.csv", attrs=...) worked while the advertised equivalent dc.annotations(data, attrs=...) raised. A carried bare table states neither attributes nor vertices, so there was nothing for the refusal to protect. The refusal now fires for a directory which holds a set or the sets, and a carried table takes what the caller states — including the dimensions, which _load_file now reads from a supplied attrs as it does from a stored one. Two tests: a carried table taking attrs=, and a carried set directory still refusing it.

@coderabbitai review

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Changed files ==='
git diff --name-only HEAD~1..HEAD || true

printf '%s\n' '=== Candidate annotation modules and tests ==='
fd -t f -e py . | rg 'annotation|annotations|table'

printf '%s\n' '=== Symbol map ==='
ast-grep outline . --items all --type function --match 'annotations|find_annotations|_load_file|read_table' || true

printf '%s\n' '=== Relevant definitions and tests ==='
rg -n -C 5 --type py \
  '^\s*(def|async def)\s+(annotations|find_annotations|_load_file|read_table)\b|dc\.annotations\s*\(|annotations\s*\(' \
  . -g '!**/.git/**' || true

printf '%s\n' '=== Latest commit diff ==='
git diff --find-renames --unified=80 HEAD~1..HEAD -- '*.py' || true

Length of output: 201934

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '=== Annotation loader dispatch and bare-file loading ==='
sed -n '390,535p' dascore/core/annotation_loader.py
sed -n '880,1015p' dascore/core/annotation_loader.py

printf '%s\n' '=== AnnotationSet attribute construction ==='
ast-grep outline dascore/core/annotations.py --items all --type function --match '_build_attrs|AnnotationSet'
rg -n -A90 -B10 --type py '^\s*def _build_attrs\b|^\s*def __init__\(' dascore/core/annotations.py

printf '%s\n' '=== Table reader callers and skip coverage ==='
rg -n -C 3 --type py '\bread_table\s*\(' . -g '!**/.git/**'
rg -n -C 3 --type py 'attrs=.*dims|dims=.*attrs|attrs=\{' tests/test_core/test_annotation_loader.py

printf '%s\n' '=== Current change status ==='
git status --short

Length of output: 31378

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

nl -ba dascore/core/annotation_loader.py | sed -n '900,990p'
nl -ba dascore/core/annotations.py | sed -n '927,951p'

Length of output: 492

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

awk 'NR >= 900 && NR <= 990 { printf "%6d  %s\n", NR, $0 }' \
  dascore/core/annotation_loader.py
awk 'NR >= 927 && NR <= 951 { printf "%6d  %s\n", NR, $0 }' \
  dascore/core/annotations.py

Length of output: 6042

---

@d-chambers I found one issue.

• [P1] Refuse conflicting dims and attrs["dims"] for bare tables. At Line 960, _declared_dims selects dims and ignores attrs["dims"]. At Line 943 in dascore/core/annotations.py, AnnotationSet then overwrites the attribute value with that selected value. For example, a carried .annotations.csv with group,time accepts dc.annotations(data, dims=("distance",), attrs={"dims": ("time",)}). It parses time as an extra instead of a dimension and silently returns a set with ("distance",). This conflicts with the stated no-precedence rule for repeated dimension declarations. Compare normalized dims and attrs["dims"] before selecting the dimensions, and raise when they differ. Add a regression test for the carried bare-table path.

_{You are interacting with an AI system.}

[d-chambers]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d624b0184d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".