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

API suggestion: name and import simplification #33

Closed
gwgundersen opened this issue Nov 14, 2018 · 3 comments
Closed

API suggestion: name and import simplification #33

gwgundersen opened this issue Nov 14, 2018 · 3 comments
Labels

Comments

@gwgundersen
Copy link
Contributor

gwgundersen commented Nov 14, 2018

I want to propose a couple changes to the Ristretto API. I'm happy to try make these changes and create a pull request if the main contributors think this is a good idea.

  1. Rather than name methods compute_rx where x is the name of the non-randomized method, what about just rx? For example, rsvd instead of compute_rsvd, rlu instead of compute_rlu. This would match libraries like NumPy, but with an r prefix to indicate it is a randomized version.

  2. Rather than importing from modules named ristretto.x where x is the name of the non-randomized method, import directly from ristretto. For example, rather than importing from ristretto.cur, import directly from ristretto.

As an example of combining these two changes, instead of,

>>> from ristretto.svd import compute_rsvd

we would have,

>>> from ristretto import rsvd
@jknox13
Copy link
Collaborator

jknox13 commented Nov 14, 2018

Thank you for the suggestion.

Originally, that was the way the API was designed. However, as a project we are moving to support Scikit-learn compatible estimators (#28) and have such chosen to rename the functions with the compute_ prefix and the compelementary classes as the upper case abbreviation (ie compute_rdmd and RDMD).

@gwgundersen
Copy link
Contributor Author

Ah, okay, that makes sense.

@erichson
Copy link
Owner

@gwgundersen thanks a lot for your suggestion. Personally, I favor the API you suggested! But, as @jknox13 said, we try to push the package into a direction so that it can be integrated into standard ML pipelines. Anyway, we always need help and contributions are very welcome. If you have any good ideas or if you like to add new features, feel free to push or to drop as a mail.

Best,
Ben

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

No branches or pull requests

3 participants