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

validate: do report HINTs from NWBI #1151

Closed
yarikoptic opened this issue Nov 4, 2022 · 8 comments
Closed

validate: do report HINTs from NWBI #1151

yarikoptic opened this issue Nov 4, 2022 · 8 comments
Assignees

Comments

@yarikoptic
Copy link
Member

  • add CLI option --severity which would take a value known to Severity and not report anything above that severity level
  • make default rendering include the severity in rendering.

see in #1031 (comment) (and file available from there in) what we see with dandi validate now and additional items which nwbinspector provides as recommendations.

@TheChymera
Copy link
Contributor

TheChymera commented Nov 7, 2022

add CLI option --severity which would take a value known to Severity and not report anything above that severity level

This I understand

make default rendering include the severity in rendering.

This I do not. What do you mean by this?

@yarikoptic
Copy link
Member Author

yarikoptic commented Nov 7, 2022

make default rendering include the severity in rendering.

This I do not. What do you mean by this?

see e.g. in #1031 (comment) nwbinspector output which groups by

0  CRITICAL
===========

...
1  BEST_PRACTICE_SUGGESTION
===========================

could be similar way groups or could be just visualized in every entry ATM (edit: e.g. how bids-validator does IIRC)

@yarikoptic
Copy link
Member Author

@TheChymera what about progress on this issue? I have ran into "desire" of limiting output only to Errors today to only realize that we still do not have --severity option.

I think we might want that Severity Enum to map to integers so we make them comparable, e.g. if severity <= Severity.ERROR. Or there is some better way to make Enum "ordered" and comparable similarly @jwodder ?

@TheChymera
Copy link
Contributor

@yarikoptic do we still need this in excess of the new --ignore option.

@yarikoptic
Copy link
Member Author

--ignore is about ignoring based on ID, it is orthogonal to the severity

@TheChymera
Copy link
Contributor

TheChymera commented Mar 8, 2023

Ok, started putting this together. However, when confronted with the practical aspect, new issues become evident which were not at all obvious in the abstract... or at least not to me.

You said:

add CLI option --severity which would take a value known to Severity and not report anything above that severity level

is this a typo or do you really want something to suppress higher severity levels? I think suppressing lower severity levels would be the more useful feature 🤔 Though of course, someone might want both. Do we want to overengineer this with --severity_max and --severity_min (when we only have 3 levels), or just trust that whoever wants this is familiar enough to list what they want to suppress via something like --ignore_severities?

Also, as I brought up on the --ignore option. Is this really something we want to do, do you have an example where piping the output wouldn't achieve the same thing with more customization potential than we could feasibly parameterize?

@yarikoptic
Copy link
Member Author

add CLI option --severity which would take a value known to Severity and not report anything above that severity level

is this a typo or do you really want something to suppress higher severity levels?

yeah, was not clear semantic. You got it right: if --severity ERROR, we should not see any HINTS . But indeed might be worth making it more explicit with naming it as --min-severity (no need for max one). (best to not mix _ and - in CLI options names IMHO)

@TheChymera
Copy link
Contributor

TheChymera commented Mar 20, 2023

Introduced in #1248

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants