-
Notifications
You must be signed in to change notification settings - Fork 3
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
update/SOF 7453 #187
base: main
Are you sure you want to change the base?
update/SOF 7453 #187
Conversation
VsevolodX
commented
Jan 11, 2025
- update: add function to create empty material
- update: add tests for empty material
- update: test to sync cell with lattice
src/py/mat3ra/made/tools/modify.py
Outdated
material: Material, | ||
vector: Optional[List[float]] = None, | ||
use_cartesian_coordinates: bool = False, | ||
material: Material, |
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.
Indentation needs to be fixed
tests/py/unit/test_material.py
Outdated
a=2.0, b=3.0, c=4.0, alpha=80.0, beta=85.0, gamma=95.0, lattice_type="TRI", name="Custom Empty" | ||
) | ||
assert material.basis.elements.values == [] | ||
assert material.basis.coordinates.values == [] |
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.
Test empty should only check the emptiness.
The actual parameter values should be in test_create.
On the material level we should just check lattice and basis, name - top-level params
For lattice/basis we should have separate tests (create, etc) that check their internal parameters
src/py/mat3ra/made/material.py
Outdated
|
||
config = {"name": name, "basis": basis_config, "lattice": lattice_config} | ||
|
||
return cls(config) |
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.
Too much code - should just be `self.create(config_with_empty_basis)
src/py/mat3ra/made/material.py
Outdated
}, | ||
} | ||
|
||
config = {"name": name, "basis": basis_config, "lattice": lattice_config} |
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.
This is code duplicate - needs to be handled at the lattice level or through self.create