-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
Overall, this is well done. However, I had some specific comments.
api.py
1.. All functions should have a doc string. You need to document the argument types and semantics, the return type (and its semantics), and describe the function. Take a look at this. Some functions comply with this, but not all (e.g., normalize in api.py).
3. In export_models_to_json, why are you creating a directory path? There may be several directory levels, you might have to create many directories. Just given an ValueError if you can't write the file.
4. Try to use "raise" to handle error reporting. If it is an internal error (e.g., an index is out of range), use RuntimeError. If it's a user problem (e.g., can't find a file), use ValueError.
cache.py
- In CacheManager.init, use cache_dir:str; don't set the default value to None. This way, the user must provide the argument.
- Your documentation in update_cache should include specifying the signature for progress_callback (argument types and return type, if any). In general, the user is not responsible for implementing the progress information. The standard approach in python is tqdm
- search_models:
- Specify the filers in the doc string
- Include a filter that matches case (as it stands, you always convert to lower case)
- Allow using regular expressions
Metadata
Metadata
Assignees
Labels
No labels