fixes #3307

[richardjgowers]

writing a trz file with dimensions=None issues a warning

Fixes #3307

Changes made in this Pull Request:

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[IAlibay]

Thanks, just the one comment [I know it's a near-zero use reader/writer, but we should really read the same way we write]

[IAlibay]

We probably should have self-consistency here, if None == np.zeros(6), then np.allclose(box, np.zeros(6)) should read in as None ? (my understanding is that currently it just sets a zero sized unitcell?)

[richardjgowers]

added a test for this, it's already implemented like this

[IAlibay]

My bad I forgot about :

mdanalysis/package/MDAnalysis/coordinates/base.py

Line 795 in 4154e63

if (self._unitcell[:3] == 0).all():

Side note - shouldn't that be an allclose [if we're comparing floats] ?

[IAlibay]

I'll approve - the only thing I'll leave up to you is whether you want to document the behaviour somewhere, maybe an entry in the class notes?

[I'm not going to request changes for it since it's a very low use class]

[richardjgowers]

I think because we're literally looking for zeros all() is ok here. This isn't a value which is computed and could be subject to rounding errors.

…

On Mon, 21 Jun 2021 at 10:51, Irfan Alibay ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In package/MDAnalysis/coordinates/TRZ.py <#3356 (comment)> : > @@ -571,6 +571,8 @@ def _write_next_frame(self, obj): if ts.dimensions is not None: unitcell = triclinic_vectors(ts.dimensions).reshape(9) else: + warnings.warn("Timestep didn't have dimensions information, " My bad I forgot about : https://github.com/MDAnalysis/mdanalysis/blob/4154e6304a38c076ef008907ed393d36ba5958a8/package/MDAnalysis/coordinates/base.py#L795 Side note - shouldn't that be an allclose [if we're comparing floats] ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3356 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACGSGB2S3DGYYSH6RRW5A2LTT4DSBANCNFSM47BGLDVA> .

[codecov]

Codecov Report

Merging #3356 (9fef9a6) into develop (a6d4ffc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #3356   +/-   ##
========================================
  Coverage    93.62%   93.62%           
========================================
  Files          176      176           
  Lines        22837    22838    +1     
  Branches      3225     3225           
========================================
+ Hits         21380    21381    +1     
  Misses        1406     1406           
  Partials        51       51           

Impacted Files	Coverage Δ
package/MDAnalysis/coordinates/TRZ.py	89.05% <100.00%> (+0.04%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6d4ffc...9fef9a6. Read the comment docs.