NeurodataWithoutBorders / nwbinspector

Tool to help inspect NWB files for compliance with NWB Best Practices
https://nwbinspector.readthedocs.io/
Other
17 stars 10 forks source link

[Feature]: Allow string input to 'importance_threshold' #192

Closed luiztauffer closed 2 years ago

luiztauffer commented 2 years ago

What happened?

When passing importance_threshold as a string, it can't be checked against Importance members. It could be specific to Python 3.8, from what I gathered googling it.

Steps to Reproduce

from nwbinspector import inspect_nwb

inspection_generator = inspect_nwb(
    nwbfile_path=file_path, 
    importance_threshold="BEST_PRACTICE_VIOLATION"
)
inspection_messages = list(inspection_generator)

Traceback

Traceback (most recent call last):
  File "convert_all.py", line 241, in <module>
    inspection_messages = list(inspection_generator)
  File "/media/luiz/storage/Github/nwbinspector/nwbinspector/nwbinspector.py", line 451, in inspect_nwb
    checks = configure_checks(
  File "/media/luiz/storage/Github/nwbinspector/nwbinspector/nwbinspector.py", line 123, in configure_checks
    if importance_threshold not in Importance:
  File "/home/luiz/anaconda3/envs/env_oconnor/lib/python3.8/enum.py", line 352, in __contains__
    raise TypeError(
TypeError: unsupported operand type(s) for 'in': 'str' and 'EnumMeta'

Operating System

Linux

Python Executable

Python

Python Version

3.8

Package Versions

No response

Code of Conduct

CodyCBakerPhD commented 2 years ago

OK so - according to the annotation typing of inspect_nwb and inspect_all this is not technically a bug, but rather the expected input to the argument is one of the actual Importance attributes

As such, your library usage code can be easily corrected via

from nwbinspector import inspect_nwb, Importance

inspection_generator = inspect_nwb(
    nwbfile_path=file_path, 
    importance_threshold=Importance.BEST_PRACTICE_VIOLATION
)
inspection_messages = list(inspection_generator)

which is the intended and currently supported approach.

That said, I'll re-name & label this issue and corresponding PR to the goal of adding a new feature allowing string inputs for this argument, which you might notice is something required of the CLI level code.

It could be specific to Python 3.8, from what I gathered googling it.

Enum indeed has very specific behavior differences going from 3.7 to 3.8, particularly when it comes to cross-type comparison errors.