search

materialsproject / emmet

Be a master builder of databases of material properties. Avoid the Kragle.
https://materialsproject.github.io/emmet/
Other
52 stars 64 forks source link

`MoleculeMetadata` set model config `allow='extra'` #857

Closed janosh closed 10 months ago

janosh commented 10 months ago

Fixes breaking test_cclib_taskdoc in https://github.com/materialsproject/atomate2/pull/548.

Failing lines that require allow='extra'.

2nd fix needed for atomate2 tests following #853.

codecov-commenter commented 10 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (2f6119b) 91.35% compared to head (9396be4) 91.35%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #857 +/- ## ======================================= Coverage 91.35% 91.35% ======================================= Files 138 138 Lines 12747 12747 ======================================= Hits 11645 11645 Misses 1102 1102 ``` | [Files](https://app.codecov.io/gh/materialsproject/emmet/pull/857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject) | Coverage Δ | | |---|---|---| | [emmet-core/emmet/core/base.py](https://app.codecov.io/gh/materialsproject/emmet/pull/857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject#diff-ZW1tZXQtY29yZS9lbW1ldC9jb3JlL2Jhc2UucHk=) | `100.00% <100.00%> (ø)` | | | [emmet-core/emmet/core/structure.py](https://app.codecov.io/gh/materialsproject/emmet/pull/857?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=materialsproject#diff-ZW1tZXQtY29yZS9lbW1ldC9jb3JlL3N0cnVjdHVyZS5weQ==) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

munrojm commented 10 months ago

The issue with this is that the metadata model is inherited by a lot of other documents. This effectively destroys validation for them. Can we not just enable extra fields on TaskDocument like we do with the VASP equivalent (https://github.com/materialsproject/emmet/blob/485638ec7bb688462eed9c35d5c443e22a84941f/emmet-core/emmet/core/tasks.py#L310)?

janosh commented 10 months ago

@munrojm That works too. I now test that extra fields are ignored and will set extra='allow' here instead.