-
Notifications
You must be signed in to change notification settings - Fork 370
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
Drop deprecations #2350
Drop deprecations #2350
Conversation
…sts that require writing new tests
One conclusion I came to is that:
|
as a special case note that |
OK - this is good to have a look at. |
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.
LGTM; it's nice that a lot of these cases just turn into throw(ArgumentError(...
with the same message, so the user is still getting helpful direction.
Yes, but I did not want to go too crazy with this, and left it where it was natural to leave and did not impact the code in a negative way (in some cases the rules of Julia dispatch made it difficult to preserve, and in 2 cases actually the |
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.
Maybe it would be worth leaving a comment to errors that are only there due to deprecations so that we can easily spot and remove them at some point in the future? Without comments, it could be hard to understand why we explicitly check for some unsupported patterns when you don't know the history of the package.
Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
What do you think of the suggestion I made above?
|
Ah - I do not know why I have missed this. I will add them. |
added |
Thank you! |
Fixes #2344
I turn depwarns to errors where it is reasonably easy to do.