search

MDAnalysis / pmda

Parallel algorithms for MDAnalysis
https://www.mdanalysis.org/pmda/
Other
31 stars 22 forks source link

Some features used in density function were deprecated #122

Closed VOD555 closed 4 years ago

VOD555 commented 4 years ago

@orbeckst @nawtrey Current density function in pmda is using density_from_Universe in MDAnalysis 1.9, but density_from_Universe will be removed in release 2.0.0 (and is already depreacted in the develpment version which we are using in the test). And this caused failures in tests in PR #121 .

We need to rewrite the function with DensityAnalysis(u, ...).run().density instead.

orbeckst commented 4 years ago

Really? I used PMDA’s DensityAnalysis for the new MDA DensityAnalysis. What is it using???

VOD555 commented 4 years ago

When running the tests for DensityAnalysis, I got this error: AttributeError: module 'MDAnalysis.core' has no attribute 'flags'. And a DeprecationWarning: density_from_Universe is deprecated! density_from_Universe will be removed in release 2.0.0. Use DensityAnalysis(u, ...).run().density instead. (The same as the things in #121 )

It was removed in https://github.com/MDAnalysis/mdanalysis/pull/2740 .

VOD555 commented 4 years ago

The problem is from line 310 in pmda/density.py metadata['time_unit'] = mda.core.flags['time_unit'] If it is not necessary to provide time unit to density function, we can just remove this line.

And when testing with the development version of MDAnalysis, I found that _set_user_grid was originally an independent function in MDA density.py (version 1.0.0), but it's now a staticmethod of MDA DensityAnalysis. As a result, we cannot import _set_user_grid directly from MDA density.

orbeckst commented 4 years ago

Let's just pin our dependency for tests to MDA < 2.0.0 – then everything should work.

IAlibay commented 4 years ago

And when testing with the development version of MDAnalysis, I found that _set_user_grid was originally an independent function in MDA density.py (version 1.0.0), but it's now a staticmethod of MDA DensityAnalysis. As a result, we cannot import _set_user_grid directly from MDA density.

Apologies, this one's on me. I hadn't considered that _set_user_grid was being used by anything downstream, so I just moved it without warning. If you can't use the classmethod here as it currently is (i.e. as DensityAnalysis._set_user_grid), then we could reconsider _set_user_grid's move?

orbeckst commented 4 years ago

The problem is from line 310 in pmda/density.py metadata['time_unit'] = mda.core.flags['time_unit']. If it is not necessary to provide time unit to density function, we can just remove this line.

Ok, yes – the flags are gone in 1.0. – I'll fix this.

orbeckst commented 4 years ago

@IAlibay – I think it's fine; it's a private method so there's no guarantee that it remains where it was. We can use the static method in PMDA.