Skip to content

Comments on Admin Api #1

@joseph-hellerstein

Description

@joseph-hellerstein

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

  1. In CacheManager.init, use cache_dir:str; don't set the default value to None. This way, the user must provide the argument.
  2. 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
  3. search_models:
    1. Specify the filers in the doc string
    2. Include a filter that matches case (as it stands, you always convert to lower case)
    3. Allow using regular expressions

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions