astropy / astroplan

Observation planning package for astronomers – maintainer @bmorris3
https://astroplan.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
198 stars 109 forks source link

`months_observable` update + fix #529

Closed tygger7 closed 1 year ago

tygger7 commented 1 year ago

changed months_observable to target_observable by adding ability to chose ('days','weeks', or 'months') to check against constraints. Default is 'month's to maintain functionality.

Also fixed _current_year_time_range as it was not including the 31st of Dec when run through months_observable (found by using 'days'). months_observable was the only function to use this so the change is simple.

tygger7 commented 1 year ago

found error in my request will recommit

bmorris3 commented 1 year ago

Hi @tygger7, thanks for this contribution. People are already using months_observable since it was released six years ago in #152, so we shouldn't remove it. If you'd like to add more functions like months_observable, you might consider opening another pull request which adds a new function that supports outputs in days/weeks/months, and then modify the contents of the months_observable function to call the more generic function that you've written. Some names to consider for the generic function could be something like observable_interval(interval='months').

tygger7 commented 1 year ago

Thank you. I didn't consider that. I'll make some changes and open a new pull_request.