Decouple Python bindings from Rust core and add Rust convenience API#38
Decouple Python bindings from Rust core and add Rust convenience API#38StoatPower wants to merge 5 commits into
Conversation
…neration code is decoupled from rust core; include it with feature flag ; goal is to have a clean rust core api for dmap that can more cleanly be consumed/wrapped by languages other than just python added comments for the rust read api added unit tests for Rust's read api
The previous rust-version of 1.63.0 no longer builds with the current dependency graph. Testing identified dependencies requiring newer toolchains and edition 2024 support. Keep the crate on edition 2021 while updating the minimum supported Rust version to 1.85.1.
|
Hi @StoatPower, this is a very welcome contribution, thank you! A few notes for discussion:
Again, thanks for all the work, I very much appreciate it! |
|
Hi again and thanks so much for taking the time to look over the PR and provide your insight! I fully agree with your point 2) that the top-level convenience functions aren't strictly necessary. If I'm being honest, I've always found it a battle to decide when and if to introduce convenience functions to any library or code base. At what point does the often subtle ergonomic boost outweigh the added maintenance concern? 🤔 My main argument for keeping them is not that they add new capability, but that they provide a single, semantic entry point for the primary API surface. From a consumer perspective, that can reduce friction and improve discoverability, while leaving some flexibility for the public API to evolve separately from the underlying implementation of each format. A few examples I can think of:
One other thing that influenced me was that the crate already exposes format-specific All that said, I should acknowledge that I'm not a native Rustacean and may be bringing expectations from other ecosystems with me. It's also entirely possible the benefits of the convenience API aren't significant enough to justify the added maintenance burden. Ultimately, I'm more than happy to defer to your direction here. The library I'm working on that consumes the Rust API will not suffer greatly without the convenience layer. Oh, and regards to point 1), I'll patch that in later today! Thank you! |
… for CI compatibility
|
I think I'm going to stand by my opinion at this time, but you raise a lot of good points and I think if I expected this to be a more heavily used crate I would be swayed. As it stands, I am not anticipating very many users of the Rust API, and the docs page (e.g.) shows all the Record type impls and functions anyways, and would be where a user should go for usage guidance anyways (I've always found docs.rs pages much more helpful than IDE autocomplete). I wouldn't claim to be an expert by any means on this, this was my first Rust project to learn the language and it was a hard fought battle to get it as tidy as it is along the way. Thanks for patching in the pyproject change, if you don't mind modifying lib.rs as well then we can merge this in and I'll draft a new release probably next week. |
|
Absolutely not a problem; I'll have the updates pushed momentarily! |
…ess surface area for project maintenance
…ts sibling . Updated the Python bindings to call it as appropriate.
|
I initially left the Once I took a closer look it felt a bit out-of-place all alone there in I've pushing another commit that removes it as a macro from If you have a chance, I'd appreciate your thoughts on it. It feels like a better fit with the direction we discussed, but I'd be interested to hear whether you think it lands in the right place or if I'm missing something. |
Hi @RemingtonRohel 👋
I'm currently working with the Virginia Tech SuperDARN lab and have been exploring use of
dmapfrom Elixir via Rustler without introducing a Python dependency (wip -> darn-dmap).That requirement motivated this PR which separates the Python bindings from the core Rust functionality so it can be consumed independently. The Python functionality is preserved while being feature gated behind the flag
--features python, passable to bothcargoandmaturin.Also added are top-level Rust convenience functions generated via macro and following the existing
dmapconventions. Unit tests are added that assert parity between the convenience function API and the internal functionality per record type.I also updated the MSRV to
rust-version = "1.85.1"to match the current dependency requirements. While testing, versions older than 1.85.1 surfaced build failures due to dependency drift.All tests are passing for both Rust (
cargo test) and Python (cargo test --features python), and the PR is fully up-to-date with your most recent work on version0.8.0.I'd be happy to make any changes or discuss alternative approaches if you have a preferred direction for the crate.
Thanks for your work on the project!
~ Matt