-
Notifications
You must be signed in to change notification settings - Fork 16
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
anesthetic 2 deprecation cleanup #320
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #320 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 34 34
Lines 2858 2820 -38
=========================================
- Hits 2858 2820 -38
☔ View full report in Codecov by Sentry. |
In light of the rather fast incrementing version number, I'd suggest giving it some more time... |
Agreed * sets reminder * |
"\'hist_1d\' and \'hist_2d\' are the appropriate " | ||
"keywords for anesthetic. Your plots may look " | ||
"odd if you use this argument." | ||
) |
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.
I don't understand why these warnings should be removed?
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.
Agreed, these will actually still be useful for new users of anesthetic. This is different from warnings that are about changes from anesthetic 1 to anesthetic 2.
So, yes, I think we should keep them, but then they should become part of the tests. Let's address this in #342.
"\'hist_1d\' and \'hist_2d\' are the appropriate " | ||
"keywords for anesthetic. Your plots may look " | ||
"odd if you use this argument." | ||
) |
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.
Agreed, these will actually still be useful for new users of anesthetic. This is different from warnings that are about changes from anesthetic 1 to anesthetic 2.
So, yes, I think we should keep them, but then they should become part of the tests. Let's address this in #342.
Description
This PR can be merged when we are happy to deprecate the guardrails for users still transferring from anesthetic 1.
Checklist:
flake8 anesthetic tests
)pydocstyle --convention=numpy anesthetic
)python -m pytest
)