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

[WIP] Import fix #321

Open
wants to merge 5 commits into
base: 1.X
Choose a base branch
from
Open

[WIP] Import fix #321

wants to merge 5 commits into from

Conversation

xivh
Copy link
Contributor

@xivh xivh commented Oct 29, 2023

I have fixed the import error caused by an existing properties.calc.json as described in #320. I fixed the segfault for empty force in #313. For an empty value field like this:

    "atom_properties": {
        "force": {
          "value": []
        }
    }

the import will fail, but there is now an error message instead of a segfault. If value is missing there will also be an error message and the import will fail. The import is already successful if there is no force field, but now I have hardcoded force to be always optional, so the import will work for the above JSON. This seems a bit risky - it would probably be better to pass a list of optional properties.

I also updated some documentation:

  • import_properties is false by default
  • changed the help string from additional_files to copy_additional_files
  • copy_additional_files requires copy_structure_files
  • copy_structure_files implies import_properties because it copies properties.calc.json

Two things that I did not address are that the copy_additional_files option prints every copied file to stdout, which is a lot of printing, and the documentation for cp_files says that it returns whether or not files were copied, but I don't think that it does.

@darjaved
Copy link

the fix contains files with the extension "cc" , but the original file are with the extension of "hh".

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.

2 participants