Fix PDBWriter with StringIO for invalid coordinates

[RMeli]

Fixes #2512

Changes made in this Pull Request:

• Ignore TypeError when PDBWriter is trying to remove a StringIO object with invalid coordinates, so that the correct exception can be raised instead.

PR Checklist

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

[codecov]

Codecov Report

Merging #2518 into develop will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2518   +/-   ##
========================================
  Coverage    90.41%   90.41%           
========================================
  Files          170      170           
  Lines        23150    23150           
  Branches      2990     2990           
========================================
  Hits         20930    20930           
  Misses        1621     1621           
  Partials       599      599           

Impacted Files	Coverage Δ
package/MDAnalysis/analysis/base.py	97.24% <0.00%> (ø)	⬆️
package/MDAnalysis/coordinates/TRZ.py	83.64% <0.00%> (ø)	⬆️
package/MDAnalysis/analysis/lineardensity.py	83.11% <0.00%> (ø)	⬆️
package/MDAnalysis/lib/util.py	88.26% <0.00%> (ø)	⬆️
package/MDAnalysis/analysis/gnm.py	96.06% <0.00%> (ø)	⬆️
package/MDAnalysis/lib/formats/libdcd.pyx	78.96% <0.00%> (ø)	⬆️
package/MDAnalysis/core/topologyobjects.py	97.87% <0.00%> (ø)	⬆️
package/MDAnalysis/analysis/waterdynamics.py	82.72% <0.00%> (ø)	⬆️
package/MDAnalysis/core/groups.py	98.24% <0.00%> (ø)	⬆️
...age/MDAnalysis/analysis/hbonds/hbond_autocorrel.py	87.50% <0.00%> (ø)	⬆️

---

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 6efe65a...6efe65a. Read the comment docs.

[jbarnoud]

This is equivalent to

except TypeError:
    pass

Shouldn't it be the following?

except TypeError:
    if isinstance(self.filename, StringIO):
        pass
    else:
        raise

[RMeli]

Definitely! Thanks for spotting this problem!

[RMeli]

The same logic should apply to the previous exception as well, which is suffering from the same problem.

However, this rises a question: should we ignore all the exceptions that can occur while trying os.remove so that the correct exception ValueError (which is the problem triggering os.remove) is thrown instead?

[jbarnoud]

We can at least wonder what would be the equivalent of an os.remove on a StringIO. Now, we have an odd discrepancy in behaviour between files and StringIO. Maybe it should be a different issue?

[RMeli]

I think the equivalent of os.remove for StringIO is just getting out of scope. However, it looks like that calling close() already frees the memory buffer:

StringIO.close()
Free the memory buffer. Attempting to do further operations with a closed StringIO object will raise a ValueError.

[jbarnoud]

I guess, then, that the handling is appropriate.