Issue #260 More python 3 compatibility fix #683
Conversation
Mostly bytes/strings issues
| @@ -171,6 +171,10 @@ def sl(): | |||
| assert_raises(TypeError, sl) | |||
|
|
|||
| def _check_getitem(self, sl): | |||
| try: | |||
| print(sl.dtype) | |||
There was a problem hiding this comment.
Oops... I'll remove it with the next commit.
jbarnoud
changed the title
| @@ -306,7 +306,7 @@ def make_density(self): | |||
| warnings.warn(msg) | |||
| return | |||
|
|
|||
| dedges = map(np.diff, self.edges) | |||
| dedges = list(map(np.diff, self.edges)) | |||
There was a problem hiding this comment.
This is a complicated way for a list comprehension [np.diff(e) for e in self.edges]
There was a problem hiding this comment.
On 29/01/16 22:49, kain88-de wrote:
In package/MDAnalysis/analysis/density.py
#683 (comment):@@ -306,7 +306,7 @@ def make_density(self):
warnings.warn(msg)
return
dedges = map(np.diff, self.edges)dedges = list(map(np.diff, self.edges))This is a complicated way for a list comprehension |[np.diff(e) for e
in self.edges]|—
Reply to this email directly or view it on GitHub
https://github.com/MDAnalysis/mdanalysis/pull/683/files#r51320581.I do not have the reflex yet.
There was a problem hiding this comment.
basically every map can be replaced with it.
Following suggestion by @richardjgowers in MDAnalysis#683 review.
Following a suggestion by @kain88-de on MDAnalysis#683 review.
|
@richardjgowers @kain88-de I pushed fixes for your comments. If the tests pass it is good for review. |
| @@ -61,7 +61,8 @@ def parse(self): | |||
| :Returns: MDAnalysis internal *structure* dict as defined here. | |||
| """ | |||
| # Open and check psf validity | |||
| with openany(self.filename, 'r') as psffile: | |||
| with openany(self.filename, 'rt') as psffile: | |||
| print(type(psffile)) | |||
|
Looks good if @kain88-de is happy too |
| @@ -116,8 +116,12 @@ def _read_offsets(self, store=False): | |||
| np.savez(offsets_filename(self.filename), | |||
| offsets=offsets, size=size, ctime=ctime) | |||
| except Exception as e: | |||
| warnings.warn("Couldn't save offsets because: {}".format( | |||
| e.message)) | |||
| try: | |||
There was a problem hiding this comment.
Did the exception format change in Py3? Is there something in six so that we don't have to write the try block everytime? How do other packages deal with this?
There was a problem hiding this comment.
It seems that some exceptions do not have a message attribute. I was receiving errors about PermissionError not having it.
There was a problem hiding this comment.
So coincidence I just triggered the exception under python 2 and got a warning that BaseException.message deprecated is. So I guess they removed in Python 3. But that means that the code in your except branch should work under python 2.7 as well.
|
Looks good to me besides the comment about the exception. |
Issue #260 More python 3 compatibility fix
Mostly bytes/strings issues
Most parser are now python 3 compatible. The main test suites that still fail are
test_streamio.pyand some analyses tests. They can potentially reveal hidden ugly stuff...When we open a file (with
openoranyopen), we must specify if it is a binary or a text file as they are handled in a different way in python 3.