Add crystal_system field to MoyoDataset
#62
Conversation
…roup number - add crystal_system field to MoyoDataset - implement crystal system mapping for space groups 1-230 - add Python bindings for CrystalSystem - update tests to verify crystal system detection for various structures
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62 +/- ##
==========================================
- Coverage 89.72% 89.19% -0.53%
==========================================
Files 44 44
Lines 5624 5665 +41
==========================================
+ Hits 5046 5053 +7
- Misses 578 612 +34 ☔ View full report in Codecov by Sentry. |
| /// The crystal system of a space group. | ||
| #[derive(Debug, PartialEq, Eq)] | ||
| #[pyclass(name = "CrystalSystem", frozen)] | ||
| pub enum PyCrystalSystem { | ||
| Triclinic, | ||
| Monoclinic, | ||
| Orthorhombic, | ||
| Tetragonal, | ||
| Trigonal, | ||
| Hexagonal, | ||
| Cubic, | ||
| } | ||
|
|
||
| impl From<CrystalSystem> for PyCrystalSystem { | ||
| fn from(cs: CrystalSystem) -> Self { | ||
| match cs { | ||
| CrystalSystem::Triclinic => Self::Triclinic, | ||
| CrystalSystem::Monoclinic => Self::Monoclinic, | ||
| CrystalSystem::Orthorhombic => Self::Orthorhombic, | ||
| CrystalSystem::Tetragonal => Self::Tetragonal, | ||
| CrystalSystem::Trigonal => Self::Trigonal, | ||
| CrystalSystem::Hexagonal => Self::Hexagonal, | ||
| CrystalSystem::Cubic => Self::Cubic, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl std::fmt::Display for PyCrystalSystem { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| write!(f, "{:?}", self) | ||
| } | ||
| } | ||
|
|
||
| #[pymethods] | ||
| impl PyCrystalSystem { | ||
| fn __str__(&self) -> String { | ||
| self.to_string() | ||
| } | ||
|
|
||
| #[classattr] | ||
| const __module__: &'static str = "moyopy"; | ||
| } |
There was a problem hiding this comment.
not sure about the code in this file. looks needlessly verbose but couldn't find another way that silences all compiler warnings... maybe due to me not knowing my way round rust yet
|
|
||
| /// The crystal system of a structure. | ||
| #[derive(Debug, PartialEq, Eq, Clone, Copy)] | ||
| pub enum CrystalSystem { |
There was a problem hiding this comment.
We already have CrystalSystem class in data/classification.rs.
moyo/moyo/src/data/classification.rs
Lines 103 to 111 in 4748c80
I am sorry that you already implemented this, but can I take over this issue? I am thinking to add a python function get_space_group_type to return crystal systems and other classifications for space group types.
#52
There was a problem hiding this comment.
ofc, feel free to take over/make any changes
There was a problem hiding this comment.
if you make it a python function, will the same information also be available on the rust side?
There was a problem hiding this comment.
The Rust side already has this information: space group type -> arithmetic crystal class -> geometric crystal class -> crystal system. So, I think the accessor for the Python interface is enough for implementation.
There was a problem hiding this comment.
ah, i should have read the code more carefully. at least i understand it better now
|
I'll close this PR because I believe |
3 participants
this PR addresses my comment over at CompRhys/aviary#96 (comment)
Summary
CrystalSystemenum to represent crystal system based on space group numberCrystalSystem