Closed xiki-tempula closed 1 year ago
@orbeckst I have done
I'm a bit reluctant to do
The AutoMBAR is only directly used in test_fep_estimators, which is now wrapped in pytest.deprecated_call
. In other parts of the code, AutoMBAR is only used once in test_workflow_ABFE so might be good to just leave it there.
The warning shows up multiple times
src/alchemlyb/tests/test_convergence.py: 2 warnings
src/alchemlyb/tests/test_fep_estimators.py: 12 warnings
src/alchemlyb/tests/test_visualisation.py: 1 warning
src/alchemlyb/tests/test_workflow_ABFE.py: 10 warnings
/home/runner/work/alchemlyb/alchemlyb/src/alchemlyb/estimators/mbar_.py:216: DeprecationWarning: From version 2.0.0, this will be replaced by the default alchemlyb.estimators.MBAR.
warn(
I'd prefer not any of our own warnings making it to the test output if we can do so. With our deprecations we know exactly what's going on. By removing the warnings, we cut down on the noise.
What's your rationale for keeping them visible?
@orbeckst I do agree that if we keep a test that triggers an unnecessary warning for an extended period of time, it is not ideal.
However, for this particular warning, It is a bit difficult to pinpoint the tests that use AutoMBAR
as they are operated via the default options, so I cannot search for them. Given that we are going to release 1.0.1 very soon and then straight to 2.0, where I will then need to remove them, I think it might not worth the time of trying to find these tests.
Can we disable them globally (eg pytest.ini) and only enable them when we want to test them?
Codecov Report
98.36% <100.00%> (+0.05%)
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.