-
Notifications
You must be signed in to change notification settings - Fork 667
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
PendingDeprecationWarning instead of DeprecationWarning #2491
Comments
This is code that is in progress of getting removed as part of #2443 , but I believe all previous cases of deprecations have been calling |
Should it be removed? The message says that it will be deprecated in version |
That is a very good point, I hadn't read it properly it seems. Maybe one of the coredevs can clarify here, I'm glad I hadn't pushed part 2 of my removals yet 😅 |
I think if we are going for
But removing code is always good, so maybe it can be removed directly in |
Yes, I agree, I'll add the change assuming v1.0 as part of the removal changes. (the alternative code seems like it was added in v0.21 so it probably is best to just go for a proper deprecation). |
In this case PendingDeprecationWarning seems more appropriate than DeprecationWarning. Is this an existing warning in 2 and 3? 1.0 will contain both hbonds modules as we want to make it easy for users to keep the last 2-capable version of MDA for their projects. For 2.0 we would want to remove hbonds — if we can. |
Yes, I checked other deprecation warnings and this is the only one where "pending" is more appropriate. |
Thanks for checking. It seems that we’re heading for 1.0 so leave it.
|
I was looking at #2472 and found out that there are two sub-packages for hydrogen bond analysis:
MDAnalysis/analysis/hbond
andMDAnalysis/analysis/hydrogenbonds
. InMDAnalysis/analysis/hbond
there is this warning:I think a
PendingDeprecationWarning
is more appropriate in this case.The text was updated successfully, but these errors were encountered: