-
Notifications
You must be signed in to change notification settings - Fork 761
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
extend exception for invalid offset_name
#3115
Conversation
That is a good point; if you do not mind, I will split it into two PRs as these springs are used in multiple places, and this PR keeps focused on the exception enrichment... So, I would take the conversion of the string just after :) |
@kashif, based on your reaction, if you are fine with keeping this PR simple, would you mind approving Ci and reviewing it? :) |
I am not a maintainer so have no such power... but I believe they will review it |
@@ -146,7 +146,7 @@ def _make_lags_for_month(multiple, num_cycles=3): | |||
+ _make_lags_for_hour(offset.n / (60 * 60)) | |||
) | |||
else: | |||
raise Exception("invalid frequency") | |||
raise ValueError(f"invalid frequency | `freq_str={freq_str}` -> `offset_name={offset_name}`") |
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.
raise ValueError(f"invalid frequency | `freq_str={freq_str}` -> `offset_name={offset_name}`") | |
raise ValueError(f"invalid frequency: {freq_str=}, {offset_name=}") |
@Borda thanks for this! Just left a comment on improving the message, let me know what you think. There’s a lot of red in the workflows, I’ll take a look and try to unblock PRs |
yes, that looks better, but seems you merged it before I could update it
|
*Description of changes:* backporting fixes - #3094 - #3115 Additionally: cap pandas version to <2.2 By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. **Please tag this pr with at least one of these labels to make our release process faster:** BREAKING, new feature, bug fix, other change, dev setup --------- Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
Issue #, if available: it is quite hard to understand why the exception was raised when you do not see the values
Description of changes: extend the exception, which will help a user understand what went wrong...
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Please tag this pr with at least one of these labels to make our release process faster: BREAKING, new feature, bug fix, other change, dev setup
cc: @jaheba, @lostella, @kashif, @rshyamsundar