Python interface improvements for GroundStation#548
Conversation
…zation Co-authored-by: ChristopherRabotin <4823784+ChristopherRabotin@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Python bindings for the GroundStation struct under the python feature flag, including serialization and deserialization capabilities for Dhall and YAML formats. The review feedback identifies a compilation error in the Dhall parsing method, redundant conditional compilation attributes, and opportunities to simplify the constructor implementation.
| fn py_from_dhall(_cls: &Bound<'_, pyo3::types::PyType>, dhall_str: &str) -> PyResult<Self> { | ||
| serde_dhall::from_str(dhall_str) | ||
| .parse() | ||
| .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string())) | ||
| } |
There was a problem hiding this comment.
serde_dhall::from_str returns a Result<Parsed, Error>. Calling .parse() directly on a Result is a compilation error. Instead, use .and_then(|parsed| parsed.parse()) to parse the inner value.
| fn py_from_dhall(_cls: &Bound<'_, pyo3::types::PyType>, dhall_str: &str) -> PyResult<Self> { | |
| serde_dhall::from_str(dhall_str) | |
| .parse() | |
| .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string())) | |
| } | |
| fn py_from_dhall(_cls: &Bound<'_, pyo3::types::PyType>, dhall_str: &str) -> PyResult<Self> { | |
| serde_dhall::from_str(dhall_str) | |
| .and_then(|parsed| parsed.parse()) | |
| .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string())) | |
| } |
| #[cfg(feature = "python")] | ||
| use super::GroundStation; | ||
| #[cfg(feature = "python")] | ||
| use pyo3::prelude::*; | ||
| #[cfg(feature = "python")] | ||
| use anise::astro::Location; | ||
| #[cfg(feature = "python")] | ||
| use indexmap::{IndexSet, IndexMap}; | ||
| #[cfg(feature = "python")] | ||
| use super::super::msr::MeasurementType; | ||
| #[cfg(feature = "python")] | ||
| use super::super::noise::StochasticNoise; | ||
| #[cfg(feature = "python")] | ||
| use hifitime::Duration; | ||
|
|
||
| #[cfg(feature = "python")] | ||
| #[pymethods] |
There was a problem hiding this comment.
Since the entire python module is conditionally compiled via #[cfg(feature = "python")] pub mod python; in mod.rs, these individual #[cfg(feature = "python")] attributes on every import and the impl block are redundant and clutter the code. They can be safely removed to improve readability.
use super::GroundStation;
use pyo3::prelude::*;
use anise::astro::Location;
use indexmap::{IndexSet, IndexMap};
use super::super::msr::MeasurementType;
use super::super::noise::StochasticNoise;
use hifitime::Duration;
#[pymethods]
impl GroundStation {
|
Visit the preview URL for this PR (updated for commit 3b07c6a): https://nyx-rustdoc--pr548-python-ground-statio-45pth7ol.web.app (expires Mon, 15 Jun 2026 21:34:18 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: d8e2a55934352d850c15d11866c39eb2d2e029be |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/md-od-python-gh-540 #548 +/- ##
===========================================================
Coverage ? 77.97%
===========================================================
Files ? 103
Lines ? 16661
Branches ? 0
===========================================================
Hits ? 12991
Misses ? 3670
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
This patch introduces a robust
__new__Python constructor forGroundStationthroughpyo3, with full field configuration capabilities. In addition, it implements Python-native serialization and deserialization via classmethods and object methods, namely:from_yaml,to_yaml,from_dhall,to_dhall,load_many_yaml,loads_many_yaml,dump_many_yaml, anddumps_many_yaml. This fulfills the user request of allowing full construction and multpile-station YAML/Dhall I/O from Python.PR created automatically by Jules for task 2480680211005972602 started by @ChristopherRabotin