Add messages#70
Conversation
| additional_metadata_file_path=additional_metadata_file_path, | ||
| ) | ||
|
|
||
| if messages is not None: |
There was a problem hiding this comment.
QUESTION: should we add a --save-messages flag to save a .json dump of the messages in the home folder (like a log file)?
There was a problem hiding this comment.
TBH I think these files will be so small we should just save them automatically (both a human-readable text output and JSON format) indexed by the input conditions (+/- unique identifier of content)
There was a problem hiding this comment.
SUGGESTION FROM YARIK: Yes, just save them, treat like 'log files'
There was a problem hiding this comment.
This will be done in a follow-up
|
From discussion nwb2bids should attempt to 'sanitize' fields in an attempt to 'harmonize' from NWB to BIDS as much as possible; these messages should inform the user that this occurred and offer a flag Examples of sanitization:
Philosophy is, if sanitization is requested, always try to force things through as much as possible, but if we hit a limit, complain. By default, no sanitization. |
|
Heudiconv uses a two-step process for editing (with suggestions via sanitization?) where basic metadata is extracted, version to be written to BIDS is written as a |
| text = ( | ||
| f"{len(messages)} suggestion for improvement was found during conversion." | ||
| if len(messages) == 1 | ||
| else f"{len(messages)} suggestions for improvement were found during conversion." | ||
| ) |
There was a problem hiding this comment.
QUESTION: right now this is the only notification that anything was less than perfect during the nwb2bids 'conversion'
Should we:
- always printout an aggregated summary here? (akin to a much smaller and more specific version of BIDS validator or NWB Inspector)
Or
- should there merely be instructions for viewing the full content? (such as, "To see these suggestions, call
nwb2bids inspect [paths]" after callingnwb2bids convert [paths])
There was a problem hiding this comment.
might be worth following bids-validator output structure for structure...
agreed to be concise and point to potentially "vast" lists of records + logs
There was a problem hiding this comment.
SUGGESTION FROM YARIK: notify user after call to nwb2bids convert that suggestions were discovered and can be seen at a certain file or printed using a certain command, similar to how DANDI-CLI notifies about log file at the end
There was a problem hiding this comment.
This will be done in a follow-up
…t init fix: debug tests
|
@candleindark Ready for first round of review Future PRs would expand the number of messages as well as allow automatic sanitization If you could also provide idea/feedback on the comments about 'QUESTIONS' some of those may also belong to follow up PRs since this is substantial as-is |
Co-authored-by: Isaac To <candleindark@users.noreply.github.com>
Co-authored-by: Isaac To <candleindark@users.noreply.github.com>
|
@candleindark Addressed comments |
|
🧹 Documentation preview for PR #70 has been removed. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
- Coverage 89.27% 87.34% -1.94%
==========================================
Files 19 24 +5
Lines 541 885 +344
==========================================
+ Hits 483 773 +290
- Misses 58 112 +54
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
3 participants
Main infrastructure along with tests and a couple small examples specific to the participants model (more to follow)