search

scikit-hep / histbook

Versatile, high-performance histogram toolkit for Numpy.
BSD 3-Clause "New" or "Revised" License
109 stars 9 forks source link

README: fix missing use of "r" and "phi" shorthands #50

Closed veprbl closed 5 years ago

veprbl commented 5 years ago

I was reading the manual and found myself questioning where did the phi and r come from. Turns out the snippet where those are defined wasn't using them, so I just didn't notice them defined.

veprbl commented 5 years ago

Also it is possible to write "-pi <= arctan2(y, x) < pi" as f"-pi <= {phi} < pi" which uses string interpolation from python 3.6+. I guess, it better to not use that in README to avoid confusion.

jpivarski commented 5 years ago

That came from a former feature in which r and phi could be Python lambdas. I relied on a package called Meta to turn Python functions into ASTs, which is what histbook uses to manipulate expressions for optimization. (The strings are immediately parsed using ast.parse.) Unfortunately, Meta doesn't seem to be well maintained and I encountered some bugs when using it in Python 3.

Also, as per these discussions with Hans (Boost.Histogram), Jan (physt), and Ben (FAST), histbook is likely to be refactored. The expression-optimization part makes more sense in awkward-array or something similar— it's not a histogramming thing.

So this r and phi came from removing examples in which they're lambdas, and I guess I didn't make the translation right. Thanks for catching it!

That's a neat Python 3.6 feature!