-
Notifications
You must be signed in to change notification settings - Fork 78
Small bugfixes #1372
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
base: develop
Are you sure you want to change the base?
Small bugfixes #1372
Conversation
|
@tsmathis if you can check that the new serde's don't impact builds, that would be super helpful! The existing logic shouldn't have changed for the serdes. I had to add a couple of catches for when subfields are partially deserialized already |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1372 +/- ##
===========================================
- Coverage 85.97% 85.96% -0.01%
===========================================
Files 229 229
Lines 17873 17888 +15
===========================================
+ Hits 15366 15378 +12
- Misses 2507 2510 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Missed the tag yesterday, will take a look first thing tomorrow! |
…e; move hubbards to model post init
tsmathis
left a comment
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.
Do you encounter an issue with the (de)serialization behavior in the electrode_adapter module?
I had written that around the outputs from the document model constructors used in the builders and being able to round trip that output, so there should really be no other entry points to that adapter module than model_dump()ing a builder output and then deserializing a model_dump?
emmet-core/emmet/core/types/pymatgen_types/electrode_adapter.py
Outdated
Show resolved
Hide resolved
emmet-core/emmet/core/types/pymatgen_types/electrode_adapter.py
Outdated
Show resolved
Hide resolved
emmet-core/emmet/core/types/pymatgen_types/electrode_adapter.py
Outdated
Show resolved
Hide resolved
Yeah in the client - if you switch between Personally I'm fine with removing the monty decoding from the client, especially since we're building the schemas to de-/serialize correctly without it |
|
I would prefer removing the monty decoding from the client then. I'd rather we make it a goal to not pollute the emmet implementations as much as possible. And as you said, what we've done with the models/types for serialization( |
|
OK I'll revert the serde changes here and see about removing the monty decoder logic from the client |
StructureMetadata, particularlydensity_atomicmaterials/summary/statsendpoint to close client issue #1033