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

Remove get_summary_dict() from InsertionElectrodeDoc #520

Merged
merged 5 commits into from
Sep 1, 2022

Conversation

acrutt
Copy link
Contributor

@acrutt acrutt commented Aug 30, 2022

The InsertionElectrodeDoc.from_entries() relied on pymatgen's InsertionElectrode.get_summary_dict(). With the most recent changes to the thermo builder (where material_ids are no longer stored as entry.entry_id), this method was no longer reliable for correctly extracting and storing material_ids. InsertionElectrodeDoc.get_elec_doc() was added to replace get_summary_dict() to resolve this bug. Overall this change will also allow other document features to be customized based on Material Project specific needs moving forward (independent of pymatgen).

Minor clean-up to InsertionElectrodeDoc.from_entries() was added to simplify this code.

@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2022

Codecov Report

Merging #520 (8f59e16) into main (07cfec2) will decrease coverage by 0.00%.
The diff coverage is 95.00%.

@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
- Coverage   97.54%   97.53%   -0.01%     
==========================================
  Files         116      116              
  Lines       24126    24135       +9     
==========================================
+ Hits        23533    23541       +8     
- Misses        593      594       +1     
Impacted Files Coverage Δ
emmet-core/emmet/core/electrode.py 96.52% <95.00%> (-0.51%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@acrutt acrutt changed the title [WIP] Remove get_summary_dict() from InsertionElectrodeDoc Remove get_summary_dict() from InsertionElectrodeDoc Aug 31, 2022
@acrutt
Copy link
Contributor Author

acrutt commented Aug 31, 2022

@munrojm I think this is ready to merge! Let me know if any edits are needed.

@munrojm munrojm added the release:patch Patch updates label Sep 1, 2022
@munrojm munrojm merged commit cd44a43 into materialsproject:main Sep 1, 2022
@acrutt acrutt deleted the ie_doc_fix branch September 1, 2022 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:patch Patch updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants