improved docstring and generalised _rescale_minmax#228
improved docstring and generalised _rescale_minmax#228kvyh wants to merge 8 commits intoastropy:masterfrom
Conversation
| what is returned for ``vals`` beyond the ``best_val`` | ||
| worse_than : 0 or 1 | ||
| what is returned for ``vals`` beyond the ``worst_val`` | ||
|
|
There was a problem hiding this comment.
Would you include an example or two demonstrating how it works? Here's an example to refer to for the format. For example, you could use altitude = [0, 10, 20] and airmass = [3, 2, 1] and show how the outputs match what you'd expect.
|
I added some examples as well as some tests for it. |
astroplan/constraints.py
Outdated
| below 35 and 1 above 60. | ||
| >>> from astroplan.constraints import _rescale_minmax | ||
| >>> import numpy as np | ||
| >>> airmasses = np.array([20, 30, 40, 45, 55, 70]) |
There was a problem hiding this comment.
I think you mean altitudes rather than airmasses here (and in the line below). Also – would you use a complete sentence in the description above, without repeating the variable definitions verbatim?
|
@bmorris3, do you have any idea why https://travis-ci.org/astropy/astroplan/jobs/151028147#L751 might be happening? |
|
I don't, but the error goes away if I restart the build. |
|
That's good, now I just need to find a fix for the |
astroplan/constraints.py
Outdated
| above = rescaled > 1 | ||
| rescaled[below] = 0 | ||
| rescaled[above] = 1 | ||
| def _rescale_minmax(vals, worst_val, best_val, better_than=1, worse_than=0): |
There was a problem hiding this comment.
I think this function should be public (remove the leading _, added to __all__) since it will be used in custom constraints.
|
Ok, I changed it to a public function and changed the description. |
|
With #232's changes, this now passes. |
|
It appears that this will be replaced by #236. |
2 participants
In the case that a user wants to create a
Constraintthat is non-boolean, they will want to use_rescale_minmaxor a similar function. This PR is trying to make the function more general and understandable by changing variable names and adding a docstring.Any ideas for variable names, or better descriptions for the docstring are welcome.