rmjarvis / TreeCorr

Code for efficiently computing 2-point and 3-point correlation functions. For documentation, go to
http://rmjarvis.github.io/TreeCorr/
Other
98 stars 37 forks source link

Make most keyword parameters invalid as positional arguments #129

Closed rmjarvis closed 3 years ago

rmjarvis commented 3 years ago

Now that I don't have to support Python 2.7 anymore, I can add a Python 3 feature I've wanted to use for a while now. Namely keyword-only arguments.

This PR converts a lot of keyword-or-positional arguments into keyword-only arguments by putting a *, before the parameters that are now keyword-only.

For instance, now, rather than dd.calculateXi(rr,dr), whose order could be easy to get wrong, you have to write dd.calculateXi(rr=rr, dr=dr). Or dd.calculateXi(dr=dr, rr=rr) would also work of course. But dd.calculateXi(dr,rr) is no longer allowed, which used to silently do the wrong thing.

Technically, the old API is still allowed, but you will get deprecation warnings telling you which arguments now have to have a keyword name, so you can convert your code over to use explicit keywords without your existing code breaking (yet). The existing syntax will work for the rest of the 4.x series, after which the new syntax will be required.

In a few functions, there are required parameters (i.e. parameters without default values) that are now keyword-only. For instance, corr.sample_pairs now has min_sep and max_sep as keyword-only. So rather than write corr.sample_pairs(n, cat1, cat2, 0.1, 100), which is unclear as to what those last two parameters mean, you now have to write corr.sample_pairs(n, cat1, cat2, min_sep=20, max_sep=100). Of course, you may also write corr.sample_pairs(n=n, cat1=cat1, cat2=cat2, min_sep=20, max_sep=100) to be even more explicit, but the first three parameters seemed like they would usually be clear from context so I let them continue to be positional if you prefer.

In all cases, the intent of this change is to try to increase the clarity of user code. If you think there are cases where this change is more burdensome than clarifying, I'd welcome feedback about it.