Skip to content
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

Waterdynamics depends on deprecated hbonds module #2745

Closed
RMeli opened this issue Jun 11, 2020 · 9 comments
Closed

Waterdynamics depends on deprecated hbonds module #2745

RMeli opened this issue Jun 11, 2020 · 9 comments
Labels
Component-Analysis remove-2.0 deprecated in 1.0 and to be removed in 2.0
Milestone

Comments

@RMeli
Copy link
Member

RMeli commented Jun 11, 2020

analysis.waterdynamics uses analysis.hbonds which is deprecated in 1.0 and scheduled for removal in 2.0 (see #2739).

@RMeli
Copy link
Member Author

RMeli commented Jun 11, 2020

Attribute timeseries is missing from the new HydrogenBondAnalysis but it is used in HydrogenBondLifetimes.run()

@orbeckst orbeckst added this to the 2.0 milestone Jun 13, 2020
@orbeckst orbeckst added the remove-2.0 deprecated in 1.0 and to be removed in 2.0 label Jun 15, 2020
@RMeli
Copy link
Member Author

RMeli commented Jun 15, 2020

@alejob @p-j-smith @MDAnalysis/coredevs I stumbled into this while trying to help @IAlibay with #2739 (see #2746 ) but I'm not very familiar with the waterdynamics.py and hbonds analysis modules. Do you have any suggestion on how to best fix this problem?

@alejob
Copy link
Member

alejob commented Jun 15, 2020

Hey there, is any replace for analysis.hbonds???

@orbeckst
Copy link
Member

Yes, analysis.hydrogenbonds, see MDAnalysis.analysis.hydrogenbonds.hbond_analysis – it still has a HydrogenBondAnalysis class but it's called a little bit differently (and it's faster).

@xiki-tempula
Copy link
Contributor

xiki-tempula commented Aug 18, 2020

My suggestion would be using the water bridge analysis module. If order=0 and update_selection=True, the water bridge analysis should give the same result as the old hbonds module.
For MDAnalysis.analysis.waterdynamics.HydrogenBondLifetimes
An example could be:

    def run(self, **kwargs):
        """Analyze trajectory and produce timeseries"""
        h_list = MDAnalysis.analysis.hbonds.WaterBridgeAnalysis(self.universe,
                                                                 self.selection1,
                                                                 self.selection2,
                                                                 distance=3.5,
                                                                 angle=120.0,
                                                                 order=0,# For hydorgne bond detection
                                                                 update_selection=True, # This is default to false in water bridge analysis
)
        h_list.run(**kwargs)
        self.timeseries = self._getGraphics(h_list.timeseries, self.t0,
                                            self.tf, self.dtmax)

I'm currently moving the WaterBridgeAnalysis from hbonds to hydrogenbonds.

@alejob
Copy link
Member

alejob commented Aug 18, 2020

I understand that HydrogenBondLifetimes will be removed from waterdynamics, so this update is not necessary anymore.

@IAlibay
Copy link
Member

IAlibay commented Aug 18, 2020

@alejob do we have a path already defined for this? We probably need to consider what the deprecation order is going to be for this. We probably can't do a straight removal for 2.0, maybe we'd have to consider deprecating in 1.x or just a stub to the replacement if it behaves the same way?

@alejob
Copy link
Member

alejob commented Aug 18, 2020

The class was already removed for v2.0, see #2842.

@IAlibay
Copy link
Member

IAlibay commented Aug 18, 2020

I see the deprecation warning got added in #2798, that makes sense.

I think the only thing leftover in waterdynamics from the old hbonds is an unused import then?

import MDAnalysis.analysis.hbonds

That'll get itself taken care of when we remove the old hbonds code, so I'm going to go ahead and close this issue unless there's any objections.

@IAlibay IAlibay closed this as completed Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component-Analysis remove-2.0 deprecated in 1.0 and to be removed in 2.0
Projects
None yet
Development

No branches or pull requests

5 participants