Skip to content
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

User notification if datasets are invalid. #1080

Merged
merged 12 commits into from
Jul 29, 2022
Merged

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

dandi/upload.py Outdated Show resolved Hide resolved
dandi/upload.py Show resolved Hide resolved
@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