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

Issue1663 #1680

Merged
merged 5 commits into from
Oct 22, 2021
Merged

Issue1663 #1680

merged 5 commits into from
Oct 22, 2021

Conversation

CharlieLaughton
Copy link
Contributor

Hi. 'Path-like' objects should now be supported as arguments to all trajectory and topology I/O methods. New tests check ability to read from, and write to, Paths in all formats. Docstrings have been edited as required.

@gph82
Copy link
Contributor

gph82 commented Oct 20, 2021

Cool! Does this mean I can use stringIO to get the PDB string /without saving to temp, as I was doing) when geom.save_pdb?

@CharlieLaughton
Copy link
Contributor Author

Cool! Does this mean I can use stringIO to get the PDB string /without saving to temp, as I was doing) when geom.save_pdb?

I don't think so - see https://stackoverflow.com/questions/44381249/treat-a-string-as-a-file-in-python

'you supplied %s' % type(filename))

topology = _parse_topology(top)
atom_indices = cast_indices(atom_indices)
with DTRTrajectoryFile(filename) as f:
with DTRTrajectoryFile(str(filename)) as f:
Copy link
Member

Choose a reason for hiding this comment

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

Is this conversion with str() correct? What if the path is a Bytes and the filename is not valid utf-8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right - I should be using os.fspath(filename) not assuming a cast to str will work. However in testing this it seems that the current mdtraj release doesn't support bytes as paths anyway:

traj = 'frame0.pdb'
import mdtraj as mdt
t = mdt.load(traj)
with open(bytes(traj, encoding='utf-8')) as f:
... data = f.read()
...

t = mdt.load(bytes(traj, encoding='utf-8'))
Traceback (most recent call last):
File "", line 1, in
File "/Users/pazcal/miniconda3/envs/py37/lib/python3.7/site-packages/mdtraj-1.9.6-py3.7-macosx-10.9-x86_64.egg/mdtraj/core/trajectory.py", line 378, in load
extensions = [_get_extension(f) for f in filename_or_filenames]
File "/Users/pazcal/miniconda3/envs/py37/lib/python3.7/site-packages/mdtraj-1.9.6-py3.7-macosx-10.9-x86_64.egg/mdtraj/core/trajectory.py", line 378, in
extensions = [_get_extension(f) for f in filename_or_filenames]
File "/Users/pazcal/miniconda3/envs/py37/lib/python3.7/site-packages/mdtraj-1.9.6-py3.7-macosx-10.9-x86_64.egg/mdtraj/core/trajectory.py", line 202, in _get_extension
(base, extension) = os.path.splitext(filename)
File "/Users/pazcal/miniconda3/envs/py37/lib/python3.7/posixpath.py", line 122, in splitext
p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not int

Does this count as a separate issue/bug?

Copy link
Member

Choose a reason for hiding this comment

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

If you make this PR use fspath, at least we’re not adding to the problem, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, done.

@CharlieLaughton
Copy link
Contributor Author

Have I broken something or is the problem elsewhere?

@rmcgibbo rmcgibbo merged commit 6bb35a8 into mdtraj:master Oct 22, 2021
@rmcgibbo
Copy link
Member

Something was just flakey. I re-ran the build, it worked, and I hit the green merge button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants