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

Better function names #77

Closed
HDembinski opened this issue Feb 20, 2020 · 8 comments
Closed

Better function names #77

HDembinski opened this issue Feb 20, 2020 · 8 comments

Comments

@HDembinski
Copy link
Member

The chosen function names are not ideal.

histplot -> hist

  • histplot is not Pythonic snake_case, it should have been hist_plot for better readability but actually...
  • since it is supposed to mimic plt.hist, it should have the same name

hist2dplot -> hist2 or hist2d

  • theplot at the end is superfluous, and again it is not proper snake_case
@andrzejnovak
Copy link
Member

I'm open to renamings, but I would want to avoid overlap with pyplot namespace in case we ever switch to monkeyclassing

@HDembinski
Copy link
Member Author

You should not try to avoid name clashes. We have namespaces to resolve the ambiguity. That's the reason everyone writes plt.hist instead of just hist, so that this package can also have a hist.

@HDembinski
Copy link
Member Author

Look at cupy, jax, etc, these re-implement the numpy interface with the exact same names and calling conventions under a different namespace, precisely so that you can do

# import numpy as np
from jax import numpy as np

# do numpy stuff

@andrzejnovak
Copy link
Member

andrzejnovak commented Feb 24, 2020 via email

@HDembinski
Copy link
Member Author

Ok, you are right, it is not exactly the same situation. If I understand correctly, you are already preparing for a possible merging of mplhep into pyplot. I think this is not going to happen very soon, and even when it is, the names will be debated again anyway. You cannot anticipate how that merging will go. The matplotlib guys could decide to place this in mpl_toolkit, then you would have an extra namespace anyway.

For now, we should use names that are easy to type. I like the idea of imitating the pyplot.hist and hist2d interface, so that it is easy to switch over and you do not have to relearn another interface, that is great. However, the following should work

mplhep.hist(*np.histogram(data))

@andrzejnovak
Copy link
Member

@HDembinski Would you find it a reasonable solution to alias the two for now? hist = histplot, hist2d = hist2dplot ?

@HDembinski
Copy link
Member Author

Yes, and to deprecate histplot and hist2dplot.

@HDembinski
Copy link
Member Author

This is the deprecation decorator I use in iminuit, feel free to copy this or allow me to make a PR with the change.
https://github.com/scikit-hep/iminuit/blob/develop/iminuit/_deprecated.py

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

No branches or pull requests

2 participants