From f658ab7d7204020168e1f5cb2c282263ca27c1b5 Mon Sep 17 00:00:00 2001 From: Gernot Bauer Date: Wed, 8 Jun 2022 16:43:31 +0200 Subject: [PATCH 1/2] expose IdentifierOption enum to Python and use it instead of a string as argument for json parsing --- build_wheel/src/lib.rs | 2 ++ src/parameter/identifier.rs | 2 +- src/python/parameter.rs | 59 +++++++++++++------------------------ 3 files changed, 23 insertions(+), 40 deletions(-) diff --git a/build_wheel/src/lib.rs b/build_wheel/src/lib.rs index 7e3dfea..7586c11 100644 --- a/build_wheel/src/lib.rs +++ b/build_wheel/src/lib.rs @@ -1,6 +1,7 @@ use feos_core::python::joback::PyJobackRecord; use feos_core::python::parameter::*; use feos_core::{Contributions, Verbosity}; +use feos_core::parameter::IdentifierOption; use pyo3::prelude::*; use pyo3::wrap_pymodule; use quantity::python::__PYO3_PYMODULE_DEF_QUANTITY; @@ -17,6 +18,7 @@ pub fn feos_core(py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; + m.add_class::()?; m.add_wrapped(wrap_pymodule!(user_defined))?; m.add_wrapped(wrap_pymodule!(cubic))?; diff --git a/src/parameter/identifier.rs b/src/parameter/identifier.rs index e06bf7c..5ac2299 100644 --- a/src/parameter/identifier.rs +++ b/src/parameter/identifier.rs @@ -4,8 +4,8 @@ use std::convert::TryFrom; use std::hash::{Hash, Hasher}; /// Possible variants to identify a substance. -#[repr(C)] #[derive(Serialize, Deserialize, Debug, Clone, Copy)] +#[cfg_attr(feature = "python", pyo3::pyclass)] pub enum IdentifierOption { Cas, Name, diff --git a/src/python/parameter.rs b/src/python/parameter.rs index 6816380..b9b87e8 100644 --- a/src/python/parameter.rs +++ b/src/python/parameter.rs @@ -545,27 +545,25 @@ macro_rules! impl_parameter { /// binary_records : numpy.ndarray[float] or List[BinaryRecord] /// A matrix of binary interaction parameters or a list /// containing records for binary interactions. - /// search_option : str, optional, defaults to "Name" - /// Identifier that is used to search substance if binary_records is - /// passed as a list. - /// One of 'Name', 'Cas', 'Inchi', 'IupacName', 'Formula', 'Smiles' + /// search_option : IdentifierOption, optional, defaults to IdentifierOption.Name + /// Identifier that is used to search binary records. #[staticmethod] - #[pyo3(text_signature = "(pure_records, binary_records, search_option='Name')")] + #[pyo3(text_signature = "(pure_records, binary_records, search_option)")] fn from_records( pure_records: Vec, binary_records: &PyAny, - search_option: Option<&str>, + search_option: Option, ) -> PyResult { let prs = pure_records.into_iter().map(|pr| pr.0).collect(); let brs = if let Ok(br) = binary_records.extract::>() { Ok(br.to_owned_array().mapv(|r| r.try_into().unwrap())) } else if let Ok(br) = binary_records.extract::>() { let brs: Vec<_> = br.into_iter().map(|br| br.0).collect(); - let io = match search_option { - Some(o) => IdentifierOption::try_from(o)?, - None => IdentifierOption::Name, - }; - Ok(<$parameter>::binary_matrix_from_records(&prs, &brs, io)) + Ok(<$parameter>::binary_matrix_from_records( + &prs, + &brs, + search_option.unwrap_or(IdentifierOption::Name), + )) } else { Err(PyErr::new::(format!( "Could not parse binary input!" @@ -628,28 +626,21 @@ macro_rules! impl_parameter { /// Path to file containing pure substance parameters. /// binary_path : str, optional /// Path to file containing binary substance parameters. - /// search_option : str, optional, defaults to "Name" + /// search_option : IdentifierOption, optional, defaults to IdentifierOption.Name /// Identifier that is used to search substance. - /// One of 'Name', 'Cas', 'Inchi', 'IupacName', 'Formula', 'Smiles' #[staticmethod] - #[pyo3( - text_signature = "(substances, pure_path, binary_path=None, search_option='Name')" - )] + #[pyo3(text_signature = "(substances, pure_path, binary_path, search_option)")] fn from_json( substances: Vec<&str>, pure_path: String, binary_path: Option, - search_option: Option<&str>, + search_option: Option, ) -> Result { - let io = match search_option { - Some(o) => IdentifierOption::try_from(o)?, - None => IdentifierOption::Name, - }; Ok(Self(Rc::new(<$parameter>::from_json( substances, pure_path, binary_path, - io, + search_option.unwrap_or(IdentifierOption::Name), )?))) } @@ -662,24 +653,19 @@ macro_rules! impl_parameter { /// E.g. [(["methane", "propane"], "parameters/alkanes.json"), (["methanol"], "parameters/alcohols.json")] /// binary_path : str, optional /// Path to file containing binary substance parameters. - /// search_option : str, optional, defaults to "Name" + /// search_option : IdentifierOption, optional, defaults to IdentifierOption.Name /// Identifier that is used to search substance. - /// One of 'Name', 'Cas', 'Inchi', 'IupacName', 'Formula', 'Smiles' #[staticmethod] #[pyo3(text_signature = "(input, binary_path=None, search_option='Name')")] fn from_multiple_json( input: Vec<(Vec<&str>, &str)>, binary_path: Option<&str>, - search_option: Option<&str>, + search_option: Option, ) -> Result { - let io = match search_option { - Some(o) => IdentifierOption::try_from(o)?, - None => IdentifierOption::Name, - }; Ok(Self(Rc::new(<$parameter>::from_multiple_json( &input, binary_path, - io, + search_option.unwrap_or(IdentifierOption::Name), )?))) } @@ -738,30 +724,25 @@ macro_rules! impl_parameter_from_segments { /// Path to file containing segment parameters. /// binary_path : str, optional /// Path to file containing binary segment-segment parameters. - /// search_option : str, optional, defaults to "Name" + /// search_option : IdentifierOption, optional, defaults to IdentifierOption.Name /// Identifier that is used to search substance. - /// One of 'Name', 'Cas', 'Inchi', 'IupacName', 'Formula', 'Smiles' #[staticmethod] #[pyo3( - text_signature = "(substances, pure_path, segments_path, binary_path=None, search_option='Name')" + text_signature = "(substances, pure_path, segments_path, binary_path, search_option)" )] fn from_json_segments( substances: Vec<&str>, pure_path: String, segments_path: String, binary_path: Option, - search_option: Option<&str>, + search_option: Option, ) -> Result { - let io = match search_option { - Some(o) => IdentifierOption::try_from(o)?, - None => IdentifierOption::Name, - }; Ok(Self(Rc::new(<$parameter>::from_json_segments( &substances, pure_path, segments_path, binary_path, - io, + search_option.unwrap_or(IdentifierOption::Name), )?))) } } From fa1232399749d855b059aac515d5edadebbe0435 Mon Sep 17 00:00:00 2001 From: Philipp Rehner Date: Fri, 10 Jun 2022 12:20:50 +0200 Subject: [PATCH 2/2] remove TryFrom and update CHANGELOG --- CHANGELOG.md | 1 + src/parameter/identifier.rs | 19 ------------------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc05647..2a572a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Made `cas` field of `Identifier` optional. [#56](https://github.com/feos-org/feos-core/pull/56) - Added type parameter to `FromSegments` and made its `from_segments` function fallible for more control over model limitations. [#56](https://github.com/feos-org/feos-core/pull/56) - Reverted `ChemicalRecord` back to a struct that only contains the structural information (and not segment and bond counts). [#56](https://github.com/feos-org/feos-core/pull/56) +- Made `IdentifierOption` directly usable in Python using `PyO3`'s new `#[pyclass]` for fieldless enums feature. [#58](https://github.com/feos-org/feos-core/pull/58) ## [0.2.0] - 2022-04-12 ### Added diff --git a/src/parameter/identifier.rs b/src/parameter/identifier.rs index 5ac2299..e15e008 100644 --- a/src/parameter/identifier.rs +++ b/src/parameter/identifier.rs @@ -1,6 +1,4 @@ -use super::ParameterError; use serde::{Deserialize, Serialize}; -use std::convert::TryFrom; use std::hash::{Hash, Hasher}; /// Possible variants to identify a substance. @@ -15,23 +13,6 @@ pub enum IdentifierOption { Formula, } -impl TryFrom<&str> for IdentifierOption { - type Error = ParameterError; - - fn try_from(s: &str) -> Result { - let lower = s.to_lowercase(); - match lower.as_str() { - "cas" => Ok(Self::Cas), - "name" => Ok(Self::Name), - "iupacname" => Ok(Self::IupacName), - "smiles" => Ok(Self::Smiles), - "inchi" => Ok(Self::Inchi), - "formula" => Ok(Self::Formula), - _ => Err(ParameterError::IdentifierNotFound(s.to_owned())), - } - } -} - /// A collection of identifiers for a chemical structure or substance. #[derive(Serialize, Deserialize, Debug, Clone, Default)] pub struct Identifier {