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

Remove numpy from default PythonInterpreter and potentially introduce PythonNumpyInterpreter #115

Closed
krinart opened this issue Oct 25, 2019 · 7 comments
Labels
Discussion enhancement New feature or request

Comments

@krinart
Copy link
Member

krinart commented Oct 25, 2019

Right now only Python uses third party library (specifically numpy) for linear algebra. This is inconsistent with:

  • our mission "with zero dependencies"
  • other languages.

The first step was to drop numpy from cases without vectors, implemented in PR #111 .

As the second step I want to suggest dropping numpy altogether from PythonInterpreter and potentially implement PythonNumpyInterpreter to use in cases where it would be beneficial.

As for the user API I see 2 options:

  1. Adding a new method export_to_python_with_numpy
  2. Adding a arameter with_numpy to an existing export_to_python method which would be False by default.

I personally think first option is better as users would have higher chances of noticing extra method than extra parameter with a default value.

@krinart krinart added enhancement New feature or request Discussion labels Oct 25, 2019
@StrikerRUS
Copy link
Member

Ooops! I just typed my comment practically about the same as stated in this issue! I definitely should use F5 more often 😃 .

I would like to introduce option 3:
3. Adding parameter with_deps to all existing methods which would be False by default.

It will unify the dependence policy for all languages.

@krinart
Copy link
Member Author

krinart commented Oct 25, 2019

Having it as a parameter would require users to see possible values in documentation, whereas having it as a separate method is self-explanatory.

Moreover if with_deps is boolean, it's not flexible enough to accommodate for use cases when we would want to implement support for multiple libraries.

If it's not boolean, we again have 2 options:

  • One parameter with_deps with multiple values, like numpy etc. The disadvantage here is the necessity of reading documentation (for users) and validating the value (for us).
  • Multiple boolean parameters like with_numpy. The disadvantage here (except from need to read documentation for users) is the need to validate those multiple params, like only one of them could be True.

Having dependency-specific methods still seems like a more clear and better API to me which doesn't require reading documentation or validating any parameters,

@StrikerRUS
Copy link
Member

Moreover if with_deps is boolean, it's not flexible enough to accommodate for use cases when we would want to implement support for multiple libraries.

My intuition was that one third-party library for linear algebra will be enough in any language. In that case using with_deps=True/False will be enough and easy to interpret.

I think that your approach with separate methods seems to be more general and I like it now. Also, it can benefit from class inheritance and allow to build different combinations of dependencies easier. And class naming schema Language[Dep1Dep2...]Interpreter looks very clear to understand from the first glance.

@krinart
Copy link
Member Author

krinart commented Oct 25, 2019

My intuition was that one third-party library for linear algebra will be enough in any language

This is how assumptions work. They always seem to make sense until they don't :)

In general I always try to make as few assumptions as possible, as every assumption creates a dependency. In this case we would create an artificial dependency "Every language will have only one implementation with linear algebra library". It seems like it makes sense (and it probably does), but why would we bring this unnecessary limitation?

Especially given that we are talking about our API, which should arguably be the most though-out component as that is exactly what users depend on and making backward incompatible changes in a well established API has never been an easy problem.

@StrikerRUS
Copy link
Member

Hi guys @krinart @izeigerman ! Have you come to any agreement? I can work on this issue.

@StrikerRUS
Copy link
Member

@izeigerman

Seems that two first steps have been done. I wonder do we really need to implement the third one, namely

potentially implement PythonNumpyInterpreter to use in cases where it would be beneficial.

?
To be honest, I don't see any particular reasons for that.

@izeigerman
Copy link
Member

@StrikerRUS
Yeah, I agree. I see no benefits either. I think we should close this. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion enhancement New feature or request
Development

No branches or pull requests

3 participants