[core] package kernels-data as a python wrapper.#475
Conversation
|
|
||
| /// A validated kernel name matching `^[a-z][-a-z0-9]*[a-z0-9]$`. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| #[derive(Debug, Clone, Eq, Hash, PartialEq)] |
There was a problem hiding this comment.
pyo3's #[pyclass(..., hash)] requires the Rust struct to implement std::hash::Hash — pyo3 delegates Python's __hash__ to the derived Rust hash.
Any preferred alternative?
| /// Returns `None` if the file does not exist in the given directory. | ||
| /// Raises `ValueError` if the file exists but cannot be parsed. | ||
| #[staticmethod] | ||
| fn load_from_variant(variant_path: PathBuf) -> PyResult<Option<Self>> { |
There was a problem hiding this comment.
I'd instead do load, which takes the full path to metadata.json and let the caller construct the path.
There was a problem hiding this comment.
Also not 100% sure if PyO3 has this conversion, but it might be worth trying
| fn load_from_variant(variant_path: PathBuf) -> PyResult<Option<Self>> { | |
| fn load_from_variant(variant_path: impl AsRef<Path>) -> PyResult<Option<Self>> { |
or variant_path: &Path.
There was a problem hiding this comment.
Yeah not supported. Kept PathBuf, which is what pyo3 officially supports for path
arguments.
|
@danieldk thanks for the review. Your comments should have been addressed now. I have also squeezed in a mypy check in the CI for |
2 participants
Part of #396. The subsequent PR will address validation in the client part of
kernels.Could we configure the access management on PyPI similar to the release workflow of ABI check?✅