Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for unitless initialization #1

Open
jochemhoorneman opened this issue Jan 3, 2025 · 2 comments
Open

Add support for unitless initialization #1

jochemhoorneman opened this issue Jan 3, 2025 · 2 comments

Comments

@jochemhoorneman
Copy link

First of all, it's great to see this crate exists now! We were thinking about running a python library inside Rust at first, which would have been quite annoying.

We want to use this crate with our robot and it seems to work just fine. The only downside is that we have to add uom and time as dependencies purely for initializing the GeomagneticField. Uom is great to make clear what the interface is, but it would be nice to have the additional option to initialize a GeomagneticField with just floats/strings as input. This could be done via a new new_unitless function or something like that.

If you want I can make a PR for it.

@budzejko
Copy link
Owner

The problem with unitless float (in new_unitless) is that it is still measured in some unit. There is no best and common way as the APIs of 3rd party libraries from WMM site are using for height: meters, km and feet (or assuming height equal zero). It depends on application context and uom gives the flexibility for the library interface.

To fit internal application context and conventions you can use thin adapter layer. I prepared another example which is closing uom dependencies in one module (wmm):
https://github.com/budzejko/world_magnetic_model/blob/main/examples/demo_newtype.rs
It is implemented as newtype pattern, but plain functions could be also used for that purpose.

In couple of lines of code we can prepare adapter for the application convention, whatever it is:
new(height_in_meters: f32, lat: f32, lon: f32, date: &str)
new(height_in_feet: u16, point: geo::Point, year: u16, month: u8)
new(point: Point3D, date: chrono::NaiveDate)
But it still depends on the application and its internal conventions.

@jochemhoorneman
Copy link
Author

jochemhoorneman commented Jan 20, 2025

Yes there are always units involved of course. My proposal is to have something like the adapter layer as a standard part (or gated behind a feature) of this crate. The adapter function(s) can have the units in the parameter names, such as in your example, or in accompanying doc comments and it will be up to the user to make sure they use it correctly. As units for such a function I would use SI-units since that is scientific and engineering standard. Or alternatively have one version with SI-units and one with imperial units. Maybe new_unitless is not an applicable name, but you get the point.

I think it is nice as a user to have the option to use either the fail-safe interface with the uom and time types, or the more error prone but simpler interface with rust std types. In the end it is a design choice if you want something like this to exist and it's your crate, so it's up to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants