Modified the ArkaneSpecies.update_xyz_string() method #1714
Conversation
Codecov Report
@@ Coverage Diff @@
## py3 #1714 +/- ##
=======================================
Coverage 41.41% 41.41%
=======================================
Files 167 167
Lines 30118 30118
Branches 6421 6421
=======================================
Hits 12473 12473
Misses 16685 16685
Partials 960 960Continue to review full report at Codecov.
|
arkane/common.py
Outdated
| xyz_list.append(str(len(self.conformer.number.value_si))) | ||
| xyz_list.append(self.label) | ||
| for number in self.conformer.number.value_si: | ||
| elements.append(getElement(int(number)).symbol) |
There was a problem hiding this comment.
Is this for loop necessary?
There was a problem hiding this comment.
sorry, it's a left-over from a different version of this block
arkane/common.py
Outdated
| elements.append(getElement(int(number)).symbol) | ||
| for number, coordinate in zip(self.conformer.number.value_si, self.conformer.coordinates.value_si): | ||
| element_symbol = getElement(int(number)).symbol | ||
| row = element_symbol + ' ' * (4 - len(element_symbol)) |
There was a problem hiding this comment.
You can use string formatting here: row = '{0:4}'.format(element_symbol)
Note that strings are left aligned by default while numbers are right aligned by default.
arkane/common.py
Outdated
| for c in coordinate: | ||
| row += '{0:14.8f}'.format(c * 1e10) # convert m to Angstrom | ||
| xyz_list.append(row) | ||
| return os.linesep.join(xyz_list) |
There was a problem hiding this comment.
I don't think you should use linesep here. File writing should automatically take care of line endings (at least in Python 3): https://docs.python.org/3/library/os.html#os.linesep
arkane/common.py
Outdated
| element_symbol = getElement(int(number)).symbol | ||
| row = element_symbol + ' ' * (4 - len(element_symbol)) | ||
| for c in coordinate: | ||
| row += '{0:14.8f}'.format(c * 1e10) # convert m to Angstrom |
There was a problem hiding this comment.
Another option (although it might be a bit harder to read):
row += '{0:14.8f}{1:14.8f}{2:14.8f}'.format(*(coordinate * 1e10).tolist())
|
Something's messed up with the coverage report. I don't think this small PR bumped the coverage by 3%... |
Now outputs a standardized form of the xyz, also compatible with Py3 Test extended accordingly
arkane/common.py
Outdated
| xyz_list.append(str(len(self.conformer.number.value_si))) | ||
| xyz_list.append(self.label) | ||
| for number in self.conformer.number.value_si: | ||
| elements.append(getElement(int(number)).symbol) |
There was a problem hiding this comment.
sorry, it's a left-over from a different version of this block
arkane/common.py
Outdated
| elements.append(getElement(int(number)).symbol) | ||
| for number, coordinate in zip(self.conformer.number.value_si, self.conformer.coordinates.value_si): | ||
| element_symbol = getElement(int(number)).symbol | ||
| row = element_symbol + ' ' * (4 - len(element_symbol)) |
arkane/common.py
Outdated
| element_symbol = getElement(int(number)).symbol | ||
| row = element_symbol + ' ' * (4 - len(element_symbol)) | ||
| for c in coordinate: | ||
| row += '{0:14.8f}'.format(c * 1e10) # convert m to Angstrom |
alongd
force-pushed
the
arkane_species_xyz
branch
from
59e0701
to
fdf0a48
Compare
Now outputs a standardized form of the xyz
Test extended accordingly
Addresses issue 1 reported in #1713
I think this PR should be merged into the py3 branch before #1713, since it doesn't break Py2 compatibility.