GeodesicBearing produces NAN when y value is outside of [-90, 90] about geo HOT 6 OPEN

I'll be conservative and say out of bounds long should also be an Err as it might indicate problems with the input data – it's better to draw attention to the problem there and then than rely on the fact that it happens to wrap correctly imo.

from geo.

Note that this is using geo v 0.26.0

from geo.

This third one actually seems reasonable to me:

#[test]
fn x_out_of_range() {
    // x is out of long rannge but will compute fine
    let x = point!( x: -361_f64 , y: 0_f64);
    let y = point!( x: 0_f64 , y: 0_f64);

    assert!(!x.geodesic_bearing(y).is_nan())
}

Longitude is just being wrapped.

e.g.

#[test]
fn x_out_of_range() {
    // x is out of long rannge but will compute fine
    let x = point!( x: -361_f64 , y: 0_f64);
    let y = point!( x: 0_f64 , y: 0_f64);

    assert!(!x.geodesic_bearing(y).is_nan())

    let x_simplified = point!( x: -1_f64 , y: 0_f64);
    assert_eq!(x.geodesic_bearing(y), x_simplified.geodesic_bearing(y))
}

That results hold true for haversine and geodesic bearing, which seems arguably reasonable to me. Do you think anything should change about that example?

The first two, with latitude out of bounds, seem more problematic - how should we handle nonsense input?

geodesic_bearing is returning f64::NaN, which is probably mostly an artifact of being ported from c++, but at least it indicates "something is wrong here". In rust a Result<f64> type would probably be more idiomatic.

But with the haversine implementation - it seems like it's just garbage output right? Is there any world where you can get a meaningful result from a latitude more than 90º?

My inclination at this point is, for both HaversineBearing and GeodesicBearing:

1. return an Err if the input latitude is out of bounds.
2. proceed as normal if the longitude is out of bounds, knowing that the math will effectively mod 360 the results.

Changing the return type would be a breaking change.

from geo.

HaversineBearing and GeodesicBearing recently superceded Bearing as a breaking change. I think returning Result<f64> would be an appropriate api change and within a short enough time period to be "stabilized." I would agree with @urschrei here—return an Err even though the calculation can continue but probably shouldn't.

from geo.

Hey, can this be assigned to me?

• Checking input parameters for lat lon
• Returning a Result<f64, Err>

Should I make a custom Err?

from geo.

Hey, I'm stuck and I need some help!
I opened PR #1161
If you could take a look and give me a hand, I would really appreciate it.

from geo.