Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #132 - CIF syntax errors from PyCifRW - upgrade to PyCifRW 4.2.1 #1073

Merged
merged 15 commits into from
Jan 24, 2018

Conversation

nmounet
Copy link

@nmounet nmounet commented Jan 23, 2018

Fix issue #132:

  • Upgrade from PyCifRW 3.6.2.1 to PyCifRW 4.2.1 (setup_requirements updated)
  • All uses of PyCifRW (CifFile) modified accordingly
  • Two news tests created for CifData (failing with the previous use of PyCifRW 3.6.2.1), for the square bracket syntax error and the long line error
  • One test on StructureData modified (avoid asserting all the CIF comments)
  • Two tests (StructureData and TrajectoryData) modified to handle better the possible absence of the PyCifRW module
  • New class HiddenPrints created in aiida.common.utils, to avoid wild printouts from external modules such as PyCifRW. Used in CifData, TrajectoryData and dataclasses tests.

@ltalirz
Copy link
Member

ltalirz commented Jan 23, 2018

Thanks for looking into this! I like the HiddenPrints class, will be useful also in other contexts.
Happy to approve once the tests pass.

for loop in node.values[dataname].loops:
loops[loop.keys()[0]] = loop.keys()
for loop in node.values[dataname].loops.values():
loops[loop[0]] = loop

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has the syntax really changed from keys to values?
I.e. is the result of this loop unchanged?

Copy link
Author

@nmounet nmounet Jan 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes (to both). Before, node.values[dataname].loops was a list, now it's a dictionary, whose values correspond to the keys of the previous version (more or less...).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a bit loopy...
anyhow, thanks, just wanted to make sure.

@nmounet nmounet merged commit 6418716 into aiidateam:develop Jan 24, 2018
@nmounet nmounet deleted the fix_132_PyCifRW_syntax branch January 24, 2018 09:35
sphuber added a commit that referenced this pull request Apr 3, 2018
This was merged into develop with PR #1073 but needs to be
back ported into release v0.11.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants