Skip to content

User notification if datasets are invalid.#1080

Merged
yarikoptic merged 12 commits intomasterfrom
bids_upload_error
Jul 29, 2022
Merged

User notification if datasets are invalid.#1080
yarikoptic merged 12 commits intomasterfrom
bids_upload_error

Conversation

@TheChymera
Copy link
Contributor

@TheChymera TheChymera commented Jul 19, 2022

Addressing: #1055 (comment)

Example:

chymera@decohost ~/src/bids-examples $ dandi upload -i dandi-staging eeg_cbm/
2022-07-19 14:17:00,460 [    INFO] Note: NumExpr detected 12 cores but "NUMEXPR_MAX_THREADS" not set, so enforcing safe limit of 8.
2022-07-19 14:17:00,460 [    INFO] NumExpr defaulting to 8 threads.
2022-07-19 14:17:01,688 [ WARNING] BIDSVersion 1.0.2 is less than the minimal working 1.7.0+012+dandi001. Falling back to 1.7.0+012+dandi001. To force the usage of earlier versions specify them explicitly when calling the validator.
2022-07-19 14:17:02,435 [   ERROR] The `.*?/CHANGES$` regex pattern file required by BIDS was not found.
2022-07-19 14:17:02,435 [    INFO] Logs saved in /home/chymera/.cache/dandi-cli/log/20220719181659Z-26640.log
Error: Found 1 BIDS dataset, which did not pass validation:
 * /home/chymera/src/bids-examples/eeg_cbm
To resolve this, perform the required changes or set the validation parameter to "skip" or "ignore".

@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.50%. Comparing base (97fe900) to head (6bf1847).
Report is 1009 commits behind head on master.

Files Patch % Lines
dandi/upload.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1080      +/-   ##
==========================================
+ Coverage   88.38%   88.50%   +0.11%     
==========================================
  Files          72       73       +1     
  Lines        9265     9306      +41     
==========================================
+ Hits         8189     8236      +47     
+ Misses       1076     1070       -6     
Flag Coverage Δ
unittests 88.50% <95.83%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TheChymera TheChymera marked this pull request as ready for review July 19, 2022 19:09
Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

some stylistic recommendations + should not raise exception if validation == 'skip'. Also adding some very basic test on some "invalid" BIDS dataset (no README?) would be good to have

@TheChymera
Copy link
Contributor Author

@yarikoptic should be done

Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

Great to see more tests added . Thanks! Left one question/suggestion.

@TheChymera
Copy link
Contributor Author

@yarikoptic :3

@yarikoptic yarikoptic added the patch Increment the patch version when merged label Jul 29, 2022
@yarikoptic
Copy link
Member

Thanks, let's proceed!

@yarikoptic yarikoptic merged commit afae8e8 into master Jul 29, 2022
@yarikoptic yarikoptic deleted the bids_upload_error branch July 29, 2022 22:20
@jwodder jwodder mentioned this pull request Aug 4, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants