Open kir0ul opened 2 years ago
PyNWB uses the docval decorator from HDMF for type checking. Using docval allows us to enforce types and standardize documentation of types (also type hints didn't exist in Python at that time). This will require some further investigation, but all the type information is already in the docval definitions for the functions and I think the easiest and most reliable way to add type hints in PyNWB would be to update the docval decorator to add that information to the functions. The source code for the docval decoratory is here https://github.com/hdmf-dev/hdmf/blob/695bd87dcb37fd8969ca510e8e35f893a6bddc61/src/hdmf/utils.py#L458 If this can be done by updating docval, then I think the code necessary to add this should not be too bad. An alternative approach could also be to have a function that looks at the docval of functions and generates the stubfiles for mypy. However, I think the more useful approach would be if the type hints can be added by docval.
@ajtritt @rly thoughts?
checking to see if this had been raised -
everything that docval can do can now be done by python typing, and beartype is a super fast drop in validator for function signatures.
couple reasons why deprecating docval
would be a good idea imo
__annotation__
and __signature__
, but this would require one to construct the python type annotations, so if that's being done you might as well just hook it up to a script that replaces @docval
doctest
is the single largest contributor to bad performance: in a 396s run of the integration tests, fully 289.8s cumulative time (72%) is spent in _check_args
. Note that that cumulative time does not include the time spent calling the function it wraps. Another way of putting that is to say that @docval
wrapping singlehandedly makes pynwb 4x slower. Each individual call is fast-ish, but when every function is wrapped in docval
, and the call trees are as deep as hdmf's, it adds up to be enormous (eg. in this run that i cancelled bc it was taking too long, there are 6.5m calls to __parse_args
, 24m calls to check_type
, 98m calls to isinstance
and so on. Most of that is from the body of __parse_args
, so there isn't really a cheap way to make that faster, there are just a lot of operations done and objects created in there.functools.lru_cache
. kwargs
func_call
happens between every actual call. imo it would be a huge improvement to both pynwb and hdmf, developer quality of life is a big determinant of contributorship, and when i can't use static analysis tools and running the tests takes 35 minutes and i spend a whole day trying to make a 20 line contribution, i'm less likely to do so, which means all the labor ends up shunted over to y'all.
fortunately it would be a pretty easy swap from what i can tell - beartype is fast and requires no additional code, and if you want to validate shapes during function calls good news because i also wrote that for ya, and that can still be memoized/cached as long as the arguments are hashable, which is the usual requirement.
Is your feature request related to a problem? Please describe. I'm trying to get a fulling working type checking pipeline on my codebase with Mypy and I'm getting the following kind of errors:
Describe the solution you'd like Not a top priority but I was wondering if you would consider adding static typing or stub files at some point?
Describe alternatives you've considered At the moment the alternative is to completely ignore PyNWB regarding type checking.
Checklist