-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
BIDS: does not upload anything BIDS if not explicitly specifying and --allow-any-path #1055
Comments
Ok, so I tracked this down. The issue I assume is the datasets lack the The “error” (which is not an error, it's a proper validation failure) is caused by the following lines, where if validation is not set to “ignore” the dataset will not be detected as valid BIDS: Lines 451 to 452 in 97fe900
Additionally, the dataset you are trying to validate uses inheritance, for which support is not yet merged upstream (and we are not yet loading the upstream code anyway): To confirm this, you can run: chymera@decohost ~/src/bids-examples $ touch asl003/dandiset.yaml
chymera@decohost ~/src/bids-examples $ dandi upload --validation=ignore -i dandi-staging asl003/
chymera@decohost ~/src/bids-examples $ touch asl003/CHANGES
chymera@decohost ~/src/bids-examples $ dandi upload -i dandi-staging asl003/ This should work. There are We have the following options:
|
According to the spec
https://bids-specification.readthedocs.io/en/stable/03-modality-agnostic-files.html#changes
it does not appear to be mandatory.
…On Tue, Jul 19, 2022, 3:06 AM Horea Christian ***@***.***> wrote:
Ok, so I tracked this down. The issue I assume is the datasets lack the
CHANGES file which BIDS insists is mandatory. The fact that the BIDS
reference data is invalid as per the BIDS schema is presumably an artifact
of the reference data being linked to the JS validator and not the new
schema-synced and schema-loading validator.
The “error” (which is not an error, it's a proper validation failure) is
caused by the following lines, where if validation is not set to “ignore”
the dataset will not be detected as valid BIDS:
https://github.com/dandi/dandi-cli/blob/97fe900730c4d3723ae094402d704505c8677826/dandi/upload.py#L451-L452
Additionally, the dataset you are trying to validate, uses inheritance,
for which support is not yet merged upstream (and we are not yet using the
upstream code anyway):
bids-standard/bids-specification#1120 (comment)
<bids-standard/bids-specification#1120 (comment)>
To confirm this, you can run:
***@***.*** ~/src/bids-examples $ touch asl003/dandiset.yaml
***@***.*** ~/src/bids-examples $ dandi upload --validation=ignore -i dandi-staging asl003/
***@***.*** ~/src/bids-examples $ touch asl003/CHANGES
***@***.*** ~/src/bids-examples $ dandi upload -i dandi-staging asl003/
This should work.
There are bids-examples datasets which have CHANGES, but by a funny
coincidence, they all use inheritance, meaning that our validator would
still fail.
We have the following options:
1. The best option, convince BIDS to drop the mandatory presence of
CHANGES. There is no need for this to be mandatory.
2. The second-best option, create empty CHANGES files on all our
archives.
3. The worst, hard-code that CHANGES is not actually required.
—
Reply to this email directly, view it on GitHub
<#1055 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABM574HOFTAOVXMPCLM54TVUZHWLANCNFSM53COAGXA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Current wording at https://bids-specification.readthedocs.io/en/stable/03-modality-agnostic-files.html#changes ### `CHANGES` Version history of the dataset (describing changes, updates and corrections) MAY be provided in the form of a `CHANGES` text file. This file MUST follow the [CPAN Changelog convention](https://metacpan.org/pod/release/HAARG/CPAN-Changes-0.400002/lib/CPAN/Changes/Spec.pod). The `CHANGES` file MUST be either in ASCII or UTF-8 encoding. so it MAY be provided, not MUST. Alerted to by @satra in dandi/dandi-cli#1055 (comment)
nicely done wise @satra, indeed I do not think it is required -- I filed bids-standard/bids-specification#1151 . @TheChymera the point then is to alert the user that "I have found BIDS dataset at location {path} but it is not valid ({len(errors)} errors detected) so it will not be considered as BIDS dataset for download" in case of |
Current wording at https://bids-specification.readthedocs.io/en/stable/03-modality-agnostic-files.html#changes ### `CHANGES` Version history of the dataset (describing changes, updates and corrections) MAY be provided in the form of a `CHANGES` text file. This file MUST follow the [CPAN Changelog convention](https://metacpan.org/pod/release/HAARG/CPAN-Changes-0.400002/lib/CPAN/Changes/Spec.pod). The `CHANGES` file MUST be either in ASCII or UTF-8 encoding. so it MAY be provided, not MUST. Alerted to by @satra in dandi/dandi-cli#1055 (comment) Co-authored-by: Stefan Appelhoff <[email protected]>
@yarikoptic I think this works pending #1164 , since I've been uploading datasets as part of the |
haven't had any issues with upload, and we have extensive tests. Given no reply to the message above I'll close this. Feel free to reopen and ping me if you encounter this again. |
I thought that #1011 should have made it magically happened to use our
dandi.utils.find_files
(while excluding VCS) if bids dataset is detected, or may be just "valid bids" paths...but nothing like that happens -- nothing gets uploaded unless I explicitly use --allow-any-paths in DANDI_DEVEL=1 mode:
The text was updated successfully, but these errors were encountered: