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 16 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
16 commits
Select commit Hold shift + click to select a range
8c6ee45
design: machine-readable validate output with store/reload
yarikoptic Mar 19, 2026
3ab4741
refactor: git mv validate.py and validate_types.py into dandi/validat…
yarikoptic Mar 19, 2026
7f80ca2
refactor: update all import paths for dandi/validate/ subpackage
yarikoptic Mar 19, 2026
ddd5c3e
refactor: move validation tests into dandi/validate/tests/
yarikoptic Mar 19, 2026
c962bf1
refactor: extract _collect_results() and _filter_results() from valid…
yarikoptic Mar 19, 2026
af45790
feat: add record_version field to ValidationResult
yarikoptic Mar 19, 2026
9db6f30
feat: add --format option to dandi validate CLI
yarikoptic Mar 19, 2026
2128a86
feat: add --output option, io.py module, and auto-save sidecar
yarikoptic Mar 19, 2026
d400cfd
feat: add --summary option to dandi validate CLI
yarikoptic Mar 19, 2026
8843d80
feat: add --load option to dandi validate CLI
yarikoptic Mar 19, 2026
69a554c
feat: persist upload validation results to sidecar JSONL
yarikoptic Mar 19, 2026
cabefcf
fix: add proper type annotations to cmd_validate helpers
yarikoptic Mar 19, 2026
adacc06
feat: auto-detect --format from --output file extension
yarikoptic Mar 19, 2026
1100de5
feat: add extended grouping options to dandi validate
yarikoptic Mar 19, 2026
2ddd5d4
feat: add --max-per-group truncation to dandi validate
yarikoptic Mar 19, 2026
efe6ec1
fix: auto-save validation sidecar for all output formats
yarikoptic Mar 21, 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.

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_sidecar_path

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

upload(
sidecar = None
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.

IDK about referring to this as a 'sidecar' since that could get confusing with BIDS language

What is meant is, specifically, might be 'persistent file recording the validation results of this Dandiset'?

Even referring to it as a 'log' could get confusing with our own log file terminology (as in, the ones that contain runtime errors rather than validations)

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.

indeed there is a clash with "sidecar" in BIDS but overall it is the same meaning, just rarely used outside of BIDS. ... but here we could just call it validation_log_path for the variable.

as for the helper , ATM no critically better name instead of current validation_sidecar_path comes to mind.

note that the validation log is not necessarily of a dandiset -- could be of nwb files AFAIK, or potentially multiple dandisets... in this case what matters is that it is associated with the same run for which there is a log.

ctx = click.get_current_context()
if ctx.obj is not None:
sidecar = validation_sidecar_path(ctx.obj.logfile)

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=sidecar,
)
Loading
Loading