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

Permit electrodes with non-TM redox active species #484

Merged
merged 1 commit into from Jul 27, 2022

Conversation

acrutt
Copy link
Contributor

@acrutt acrutt commented Jul 27, 2022

There are valid electrodes that contain redox active species which are not transition metals (e.g. Sn, Sb, Bi, C). The InsertionElectrodeDocument had a check which assumes the presence of transition metals in the host material which created an error for any electrodes that contained non-transition-metal redox active species.

An if statement has been added to check for the presence of a transition metal to avoid this error and a warning has been added ("Transition metal not found") instead. Now the InsertionElectrodesBuilder can successfully process electrodes that only contain Sn, Sb, Bi, or C as the redox active specie.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #484 (55ccc9c) into main (b7b458e) will decrease coverage by 0.00%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##             main     #484      +/-   ##
==========================================
- Coverage   97.63%   97.62%   -0.01%     
==========================================
  Files         115      115              
  Lines       23900    23902       +2     
==========================================
+ Hits        23334    23335       +1     
- Misses        566      567       +1     
Impacted Files Coverage Δ
emmet-core/emmet/core/electrode.py 97.03% <60.00%> (-0.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 857811e...55ccc9c. Read the comment docs.

@munrojm munrojm added the release:patch Patch updates label Jul 27, 2022
@munrojm munrojm merged commit 767ef92 into materialsproject:main Jul 27, 2022
@acrutt acrutt deleted the tm_check branch July 27, 2022 21:19
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.

None yet

3 participants