-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[ENH] Extract cached ForecastingHorizon
methods to functions and avoid B019 error
#2364
[ENH] Extract cached ForecastingHorizon
methods to functions and avoid B019 error
#2364
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
Non-blocking comments:
- might be worth to add to the comments preceding the new, losse functions, a refrence to the B019, or someone might get the idea to put them back
- you remember the
pandas
bug? I think we should maybe change the comments there from "replace when fixed" to "replace when we upgrade our lower pandas bound to a version where this is fixed"? Or we´ll have the bug back on the lower end of our supported versions...
@fkiraly Addressed both of your suggestions. |
Thanks! May I ask for reformulation of this:
The problem with this is that it's obsccure. Would suggest to change to "This function needs to be outside |
@fkiraly I change the wording as you proposed |
readthedocs is fine (was before), but failing since too many readthedocs jobs are running concurrently |
Reference Issues/PRs
closes #2338
What does this implement/fix? Explain your changes.
Extract
ForecastingHorizon.to_relative
andForecastingHorizon.to_absolute
to_to_relative
and_to_absolute
, respectively, to allow caching without B019 error.Does your contribution introduce a new dependency? If yes, which one?
No
What should a reviewer concentrate their feedback on?
Any other comments?
PR checklist
For all contributions