-
Notifications
You must be signed in to change notification settings - Fork 192
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
Added element X to the elements list in order to support unknown species #1613
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1613 +/- ##
===========================================
+ Coverage 57.14% 57.17% +0.02%
===========================================
Files 273 273
Lines 33773 33773
===========================================
+ Hits 19300 19309 +9
+ Misses 14473 14464 -9
Continue to review full report at Codecov.
|
@giovannipizzi I don't see a problem with this, but I would like your approval. |
Indeed it might be sufficient. Could you also update the docstrings in the same file (for some reason the sentence stops...), and Great if you could also check the current tests for StructureData, and try to duplicate all of them to use X and check that everything still works |
Most of the tests duplicated (including ASE test). Tests are successful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Can be useful in parsing scenarios when the element is now known, or in cases where one does not explicitly handle atoms (electrons, orbital calculations etc.). It is then also possible to detect element X down the line and request user for additional information or do automatic corrections, if needed.