-
Notifications
You must be signed in to change notification settings - Fork 646
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
inconsistent storage of times
and frames
attributes in AnalysisBase
classes
#4617
Comments
Hi @edisj, In the rog = AnalysisFromFunction(radgyr, u.trajectory,
protein, protein.masses,
total_mass=np.sum(protein.masses))
rog.run()
print(rog.times, rog.frames) So the However, in the mdanalysis/package/MDAnalysis/analysis/base.py Lines 539 to 540 in 347a0c0
I agree this inconsistency might be confusing.
Since this is your expectation, would you find it better to have them in |
Hi @RMeli, thanks for the clarification! I ended up just adding them in my conclude method like this and it works exactly how I wanted def _conclude(self):
self.results.frames = self.frames
self.results.times = self.times
Yes, personally I think what I naturally think "times" and "frames" mean in the context of an analysis are the time values and frame numbers that correspond to the results of the analysis, so in some sense they are kind of like implicit results The documentation from
is more in line with how I think it should behave versus
What do you think? |
I found it confusing that Removing times and frames from AnalysisBase is a huge break. Adding them to It would be interesting to hear other opinions. @PicoCentauri you have looked at AnalysisBase a lot – any opinions? @marinegor are there any implications for parallel analysis? EDIT: I had confused AnalysisBase and AnalysisFromFunction |
@orbeckst from my understanding it's the other way around? AnalysisBase doesn't have times and frames is results but AnalysisFromFunction does? I think my 2 cents here is that |
Probably not, though if frames aren't numpy ndarrays, it could influence serialization. But in general, I also oppose adding them to results since they aren't results, and were there even before the run started. |
The majority opinion appears to be to deprecate |
I'm pretty new to using
AnalysisBase
to write my own custom analysis classes, and I found it confusing thatframes
andtimes
are handled differently in different classes.Using the
NewAnalysis
example in the docs (https://docs.mdanalysis.org/stable/documentation_pages/analysis/base.html#MDAnalysis.analysis.base.AnalysisBase),if you run
NewAnalysis.run()
,frames
andtimes
will be inNewAnalysis.frames
andNewAnalysis.times
.If you use
AnalysisFromFunction
, thenframes
andtimes
will be inresults.frames
andresults.times
, even though they both useAnalysisBase
Is this intended?
Expected behavior
I expected to find frames and times in
results.frames
andresults.times
in both casesActual behavior
results.frames
throwsAttributeError: 'Results' object has no attribute 'frames'
when inheriting fromAnalysisBase
, but not when usingAnalysisFromFunction
Current version of MDAnalysis
python -c "import MDAnalysis as mda; print(mda.__version__)"
)2.7.0
python -V
)?3.11.9
ubuntu 22.04
The text was updated successfully, but these errors were encountered: