Skip to content

ENH: Machine-readable validate output with store/reload #1822

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.

Sign up for GitHub

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

Open
yarikoptic wants to merge 23 commits into
base: master
Choose a base branch
from
Open

ENH: Machine-readable validate output with store/reload #1822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1e46e6b
refactor: git mv validate.py and validate_types.py into dandi/validat…
yarikoptic Mar 19, 2026
0b1d1ca
refactor: update all import paths for dandi/validate/ subpackage
yarikoptic Mar 19, 2026
20a97a5
refactor: move validation tests into dandi/validate/tests/
yarikoptic Mar 19, 2026
abedeed
refactor: extract _collect_results() and _filter_results() from valid…
yarikoptic Mar 19, 2026
9844920
design: machine-readable validate output with store/reload
yarikoptic Mar 19, 2026
113a812
feat: add record_version field to ValidationResult
yarikoptic Mar 19, 2026
27ed62c
feat: add --format option to dandi validate CLI
yarikoptic Mar 19, 2026
0bcf52b
feat: add --output option, io.py module, and auto-save sidecar
yarikoptic Mar 19, 2026
1e231f0
feat: add --summary option to dandi validate CLI
yarikoptic Mar 19, 2026
17436de
feat: add --load option to dandi validate CLI
yarikoptic Mar 19, 2026
3873319
feat: persist upload validation results to sidecar JSONL
yarikoptic Mar 19, 2026
a66ff59
fix: add proper type annotations to cmd_validate helpers
yarikoptic Mar 19, 2026
8c03dd3
feat: auto-detect --format from --output file extension
yarikoptic Mar 19, 2026
5bd0f31
feat: add extended grouping options to dandi validate
yarikoptic Mar 19, 2026
dae0de5
feat: add --max-per-group truncation to dandi validate
yarikoptic Mar 19, 2026
f07d5e5
fix: auto-save validation sidecar for all output formats
yarikoptic Mar 21, 2026
8a30f1b
refactor: address PR #1822 review feedback
yarikoptic Mar 29, 2026
e2bd9ab
refactor: rename "sidecar" to "companion" to avoid BIDS confusion + s…
yarikoptic Mar 30, 2026
c8e8f9e
test: add parametrized CLI tests for single and composite groupings
yarikoptic Apr 1, 2026
2a0169f
refactor: merge write/append_validation_jsonl into single function
yarikoptic Apr 2, 2026
9881001
DOC: add DEVELOPMENT.md about doc for doclass attributes
yarikoptic Apr 2, 2026
87d4c81
refactor: rename validate submodules to private (_io, _core, _types)
yarikoptic Apr 3, 2026
1c235d0
refactor: update imports for private validate submodules, add __all__
yarikoptic Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,108 changes: 1,108 additions & 0 deletions .specify/specs/validate-machine-readable-output.md
View file
Open in desktop

Large diffs are not rendered by default.

25 changes: 25 additions & 0 deletions DEVELOPMENT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,31 @@ instance as `dandi-api-local-docker-tests`. See the note below on the
`DANDI_DEVEL` environment variable, which is needed in order to expose the
development command line options.

## Code style conventions

### Dataclass and attrs field documentation

Document dataclass/attrs fields using `#:` comments above the field, not
docstrings below. This is the format Sphinx autodoc recognizes for attribute
documentation:

```python
@dataclass
class Movement:
"""A movement/renaming of an asset"""

#: The asset's original path
src: AssetPath
#: The asset's destination path
dest: AssetPath
#: Whether to skip this operation because an asset already exists at the
#: destination
skip: bool = False
```

See [dandi.move.Movement on RTD](https://dandi.readthedocs.io/en/latest/modref/generated/dandi.move.html#dandi.move.Movement)
for a rendered example.

## Environment variables

- `DANDI_DEVEL` -- enables otherwise hidden command line options, such as
Expand Down
2 changes: 1 addition & 1 deletion dandi/bids_validator_deno/_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pydantic import DirectoryPath, validate_call

from dandi.utils import find_parent_directory_containing
from dandi.validate_types import (
from dandi.validate._types import (
Origin,
OriginType,
Scope,
Expand Down
11 changes: 9 additions & 2 deletions dandi/cli/cmd_upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,21 @@ def upload(
can point to specific files you would like to validate and have uploaded.
"""
# Avoid heavy imports by importing with function:
from ..upload import upload
from ..upload import upload as upload_
Copy link
Copy Markdown
Contributor

@CodyCBakerPhD CodyCBakerPhD Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better of course than clobbering the namespace, but in the long term a better name that distinguishes API vs CLI functions (with CLI usually marked private since it is odd to expose them to library) is still preferred

Which would be a breaking change, not saying do it here, just thinking out loud

Copy link
Copy Markdown
Member Author

@yarikoptic yarikoptic Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah -- typically we just clobbered click's interfaces (sorry didn't interleave... but at least manually pruned some unrelated)

❯ grep -l 'from \.\.\(.*\) import \1' dandi/cli/cmd_* | xargs grep '^def '
dandi/cli/cmd_delete.py:def delete(paths, skip_missing, dandi_instance, force, devel_debug=False):
dandi/cli/cmd_download.py:def download(
dandi/cli/cmd_move.py:def move(
dandi/cli/cmd_organize.py:def organize(
dandi/cli/cmd_upload.py:def upload(
dandi/cli/cmd_validate.py:def validate_bids(
dandi/cli/cmd_validate.py:def validate(
❯ grep 'from \.\.\(.*\) import \1' dandi/cli/cmd_*
dandi/cli/cmd_delete.py:    from ..delete import delete
dandi/cli/cmd_download.py:    from .. import download
dandi/cli/cmd_move.py:    from .. import move as move_mod
dandi/cli/cmd_organize.py:    from ..organize import organize
dandi/cli/cmd_upload.py:    from ..upload import upload
dandi/cli/cmd_validate.py:from ..validate import validate as validate_

but frankly and unfortunately here it doesn't matter much since those click functions are not usable as python interfaces! I wish it was otherwise. So, no point of giving them any special names really.

Copy link
Copy Markdown
Contributor

@CodyCBakerPhD CodyCBakerPhD Mar 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but frankly and unfortunately here it doesn't matter much since those click functions are not usable as python interfaces!

How do you mean? Any other library can import them and manipulate the click groups; sometimes that might even be intentional, but I don't think so here

from dandi.cli.command import main
from dandi.cli.cmd_upload import upload  # Implying it is not private and intended to be imported and customized

import click

@main.command("upload2")  # Or even re-register under the same name if attempting some nasty injection
@click.pass_context
def wrapped_original(ctx):
    click.echo("Before original")  # Inject custom code here
    ctx.invoke(upload)
    click.echo("After original")  # Inject more custom code here

from ..validate._io import validation_companion_path

if jobs_pair is None:
jobs = None
jobs_per_file = None
else:
jobs, jobs_per_file = jobs_pair

upload(
ctx = click.get_current_context()
companion = (
validation_companion_path(ctx.obj.logfile) if ctx.obj is not None else None
)

upload_(
paths,
existing=existing,
validation=validation,
Expand All @@ -115,4 +121,5 @@ def upload(
jobs=jobs,
jobs_per_file=jobs_per_file,
sync=sync,
validation_log_path=companion,
)
Loading
Loading