search

MDAnalysis / mdanalysis

MDAnalysis is a Python library to analyze molecular dynamics simulations.
https://mdanalysis.org
Other
1.3k stars 648 forks source link

anyopen should respect binary/text arguments #2130

Open richardjgowers opened 5 years ago

richardjgowers commented 5 years ago

MDAnalysis.lib.util.anyopen (and its context manager cousin openany) currently opens files/archives/streams seamlessly and is used in most Readers. It accepts a mode argument like the regular open function, but this mode doesn't explicitly support more complex modes ie r+.

In PR #2127 we started to require using mode='rb' to get ascii reading working in Windows environments. This works currently, but isn't technically supported, ie the api for anyopen doesn't say this should work (and it's therefore not tested to work either).

These more complicated modes should be accepted & enforced (or at least a subset which we need to use, ie 'rb' for backends (ie files/archives/streams) which support this).

orbeckst commented 5 years ago

See code for anyopen().

orbeckst commented 5 years ago

We set the default mode="rt" https://github.com/MDAnalysis/mdanalysis/blob/89b2299e3ae8ab548fcc932b72eb6128b2be0463/package/MDAnalysis/lib/util.py#L333 and then say that we ignore everything beyond the first letter – that's rather inconsistent :-p.

tylerjereddy commented 5 years ago

Yes, should be clarified / consistent / tested, but useful that it is actually apparently possible for Windows of course.

jbarnoud commented 5 years ago

We also explicitly use 'rb' for the TRZ reader. I think I remember the 'rt' default was set because of python3 because some compressed files were open in byte mode. It starts to be an old change so I can be wrong.

orbeckst commented 5 years ago

Can we define clearly what needs to happen? Something like

Anything else?

richardjgowers commented 5 years ago

That sounds good to me On Mon, Nov 26, 2018 at 7:19 PM, Oliver Beckstein notifications@github.com wrote:

Can we define clearly what needs to happen? Something like

  • allow 'rb' and 'wb' (and 'rt' and 'wt') in addition to 'r', 'w', 'a'.
  • document the allowed modes
  • add checks that only allowed modes are accepted and raise ValueError for everything else
  • add tests

Anything else?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MDAnalysis/mdanalysis/issues/2130#issuecomment-441850137, or mute the thread https://github.com/notifications/unsubscribe-auth/AI0jBx6cplTFxVgTuCI3Femdsggdra9Uks5uzIUNgaJpZM4YJwgA .