search

lab-cosmo / i-pi-dev_archive

Development version of i-PI
21 stars 12 forks source link

Fix #179 #180

Closed grhawk closed 7 years ago

grhawk commented 7 years ago

Fixing #179

Thank you @OndrejMarsalek to pointing out this issues... The problem about "Angstrom" vs "angstrom" was actually due to a regex...

ceriottm commented 7 years ago

I would not capitalize angstrom in the code since all units are lowercase.

On 21 April 2017 at 21:08, Ondrej Marsalek notifications@github.com wrote:

@OndrejMarsalek approved this pull request.

Looks good to me, thanks for the quick fix!

In ipi/utils/io/backends/io_pdb.py https://github.com/cosmo-epfl/i-pi-dev/pull/180#discussion_r112757617:

@@ -116,6 +116,12 @@ def read_pdb(filedesc, **kwargs):

skip the comment field

comment = copy.copy(header) header = filedesc.readline()

  • if 'positions{' not in comment:
  • comment = comment.strip()
  • comment += ' positions{angstrom}\n'

I would still capitalize "Angstrom", at least in the code, even if it does not influence functionality.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cosmo-epfl/i-pi-dev/pull/180#pullrequestreview-34101765, or mute the thread https://github.com/notifications/unsubscribe-auth/ABESZyvtY8I9pllMyHGxJx8FYoyMd_6Iks5ryP6QgaJpZM4NEN3u .

OndrejMarsalek commented 7 years ago

Fair. I guess it just looks weird because normally you only write A and not the whole thing. Anyway, merge?