Skip to content

Fix py3 compatibility regarding the HydrogenBondAnalysis.save_table() #1744

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.

Sign up for GitHub

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

Closed
xiki-tempula wants to merge 2 commits into from
Closed

Fix py3 compatibility regarding the HydrogenBondAnalysis.save_table() #1744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dde6584
Fix py3 compatibility regarding the HydrogenBondAnalysis.save_table()
xiki-tempula Dec 18, 2017
1571731
change the change log.
xiki-tempula Dec 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package/CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ Enhancements
* introduce the water bridge analysis module (PR #1722)
* Universe.add_TopologyAttr now accepts strings to add a given attribute
to the Universe (Issue #1092, PR #1186)
* Fix py3 compatibility regarding the HydrogenBondAnalysis.save_table()
(Issue #1743, PR #1744)

Deprecations
* HydrogenBondAnalysis detect_hydrogens=heuristic is marked for deprecation in 1.0
Expand Down
2 changes: 1 addition & 1 deletion package/MDAnalysis/analysis/hbonds/hbond_analysis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,7 @@ def save_table(self, filename="hbond_table.pickle"):
"""
if self.table is None:
self.generate_table()
with open(filename, 'w') as f:
with open(filename, 'wb') as f:
cPickle.dump(self.table, f, protocol=cPickle.HIGHEST_PROTOCOL)
Copy link
Member

@kain88-de kain88-de Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not good for python 2/3 compatibility. It means that a pickle written with python 3 won't be readable with python 2. What is the type of table? Is it a numpy array? Then we have more portable options.

Copy link
Contributor Author

@xiki-tempula xiki-tempula Dec 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kain88-de Thank you for the comment. I have tested reading and writing in py3 but only tested checked in py2.
This is a numpy recarray. I guess np.save() might be a good option?

Copy link
Member

@kain88-de kain88-de Dec 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use np.savetxt then a user can consume the table however he likes. But this is a deprecation. So if this is read from somewhere we should allow reading a pickle for some time. Maybe even writing a pickle is OK until a version 1.0 (If the user really wants to).


def _has_timeseries(self):
Expand Down