-
Notifications
You must be signed in to change notification settings - Fork 23
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
Oxidation States 24 #329
Oxidation States 24 #329
Changes from 20 commits
da611fb
b3c544f
fbd1be3
ba132de
0be9b0b
e062c1c
09ebea0
1e668fc
ec8120e
3875854
bf75145
ef94fcb
88e83ac
e580d58
c1642ab
49e7568
41d7bbb
13e6eb9
b8982be
9125edb
99acd4e
3c73285
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,12 @@ | |
|
||
A collection of fast screening tools from elemental data | ||
""" | ||
|
||
import itertools | ||
import warnings | ||
from math import gcd | ||
from operator import mul as multiply | ||
from os import path | ||
from typing import Iterable, List, Optional, Sequence, Tuple, Union | ||
from typing import Iterable, List, Optional, Tuple, Union | ||
|
||
import pandas as pd | ||
|
||
|
@@ -55,12 +54,14 @@ class Element: | |
|
||
Element.oxidation_states_sp (list) : List of oxdation states recognised by the Pymatgen Structure Predictor | ||
|
||
Element.oxidation_states_icsd (list) : List of oxidation states that appear in the ICSD | ||
Element.oxidation_states_icsd (list) : List of oxidation states that appear in the 2016 version of the ICSD | ||
|
||
Element.oxidation_states_wiki (list): List of oxidation states that appear wikipedia (https://en.wikipedia.org/wiki/Template:List_of_oxidation_states_of_the_elements) Data retrieved: 2022-09-22 | ||
|
||
Element.oxidation_states_custom (list | None ): List of oxidation states that appear in the custom data file supplied (if any) | ||
|
||
Element.oxidation_states_icsd24 (list): List of oxidation states that appear in the 2024 version of the ICSD | ||
|
||
Element.coord_envs (list): The allowed coordination enviroments for the ion | ||
|
||
Element.covalent_radius (float) : Covalent radius of the element | ||
|
@@ -169,6 +170,10 @@ def __init__( | |
"oxidation_states_wiki", | ||
data_loader.lookup_element_oxidation_states_wiki(symbol), | ||
), | ||
( | ||
"oxidation_states_icsd24", | ||
data_loader.lookup_element_oxidation_states_icsd24(symbol), | ||
), | ||
Comment on lines
+200
to
+203
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Implementation for ICSD24 oxidation states is missing The verification reveals critical issues:
This change cannot be safely integrated without:
🔗 Analysis chainVerify data loader implementation and error handling The initialization of the new oxidation states attribute follows the established pattern. However, as this is a breaking change, please ensure:
Let me gather more information about the data loader implementation for the new ICSD24 function. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check data loader implementation
ast-grep --pattern 'def lookup_element_oxidation_states_icsd24($_) {
$$$
}'
# Check for any existing code that might be affected
rg -A 3 "oxidation_states_icsd\W"
Length of output: 1925 Script: #!/bin/bash
# Check implementation of the new ICSD24 function
ast-grep --pattern 'def lookup_element_oxidation_states_icsd24($_) {
$$$
}'
# Check for the data file
fd "oxidation_states_icsd24.txt"
# Check for any error handling patterns in data_loader
rg -A 3 "try:|except:|raise" smact/data_loader.py
Length of output: 243 |
||
("dipol", dataset["dipol"]), | ||
("pauling_eneg", dataset["el_neg"]), | ||
("SSE", sse), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
# | ||
# Oxidation state set | ||
# Source: Original SMACT set manually compiled (2014) | ||
# | ||
H -1 +1 | ||
He | ||
Li +1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
# | ||
# Oxidation state set | ||
# Source: Pymatgen structure predictor (2017) | ||
# | ||
H 1 | ||
He | ||
Li 1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
# | ||
# Oxidation state set | ||
# Source: ICSD (2016) | ||
# | ||
H -1 1 | ||
He | ||
Li 1 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
# | ||
# Oxidation state set | ||
# Source: ICSD (2024, V2), most common non-zero values | ||
# | ||
H 1 | ||
He | ||
Li 1 | ||
Be 2 | ||
B 3 | ||
C 4 | ||
N -3 | ||
O -2 | ||
F -1 | ||
Ne | ||
Na 1 | ||
Mg 2 | ||
Al 3 | ||
Si 4 | ||
P 5 | ||
S -2 | ||
Cl -1 | ||
Ar | ||
K 1 | ||
Ca 2 | ||
Sc 3 | ||
Ti 4 | ||
V 5 | ||
Cr 3 | ||
Mn 2 | ||
Fe 3 | ||
Co 2 | ||
Ni 2 | ||
Cu 2 | ||
Zn 2 | ||
Ga 3 | ||
Ge 4 | ||
As 5 | ||
Se -2 | ||
Br -1 | ||
Kr 2 | ||
Rb 1 | ||
Sr 2 | ||
Y 3 | ||
Zr 4 | ||
Nb 5 | ||
Mo 6 | ||
Tc 7 | ||
Ru 4 | ||
Rh 3 | ||
Pd 2 | ||
Ag 1 | ||
Cd 2 | ||
In 3 | ||
Sn 4 | ||
Sb 3 | ||
Te -2 | ||
I -1 | ||
Xe 6 | ||
Cs 1 | ||
Ba 2 | ||
La 3 | ||
Ce 3 | ||
Pr 3 | ||
Nd 3 | ||
Pm 3 | ||
Sm 3 | ||
Eu 3 | ||
Gd 3 | ||
Tb 3 | ||
Dy 3 | ||
Ho 3 | ||
Er 3 | ||
Tm 3 | ||
Yb 3 | ||
Lu 3 | ||
Hf 4 | ||
Ta 5 | ||
W 6 | ||
Re 7 | ||
Os 5 | ||
Ir 4 | ||
Pt 4 | ||
Au 1 | ||
Hg 2 | ||
Tl 1 | ||
Pb 2 | ||
Bi 3 | ||
Po 4 | ||
At | ||
Rn | ||
Fr | ||
Ra 2 | ||
Ac 3 | ||
Th 4 | ||
Pa 5 | ||
U 6 | ||
Np 6 | ||
Pu 3 | ||
Am 3 | ||
Cm 3 | ||
Bk 3 | ||
Cf 3 | ||
Es 3 | ||
Fm | ||
Md | ||
No | ||
Lr |
Large diffs are not rendered by default.
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.
💡 Codebase verification
Removal of
Sequence
import may cause issuesThe removal of
Sequence
fromsmact/__init__.py
could be problematic as it's actively used instructure_prediction/database.py
with type hints. Sincedatabase.py
importsSequence
directly fromtyping
, the removal is safe, but we should maintain consistency in import patterns across the codebase.structure_prediction/database.py
usesSequence[SmactStructure]
type hintsSequence
import in__init__.py
for API consistency🔗 Analysis chain
Verify removal of Sequence import
The removal of the unused
Sequence
import is good practice. However, as this is a public API, we should verify that this doesn't affect downstream code that might be importing it from this module.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 94
Script:
Length of output: 250