GeodesicBearing produces NAN when y value is outside of [-90, 90]

[JosiahParry]

When using geodesic_bearing() method with a point where the latitude is outside of the coordinate range [-90, 90], the result will be NaN. However, there is no corresponding result when a longitude value falls outside of the range [-180, 180]. I understand that values that fall outside of these ranges are likely to be planar coordinates and are not intended to work. If that is the case, I suspect the result of this method should return Option<f64> instead of f64 to better handle these cases.

On the contrary, the haversine_bearing() method has no such NaN behavior. It will return an f64 for any value provided despite it only being intended for values that fall within the [-180, 180] x range and [-90, 90] y range.

Below are reproducible examples:

use geo::{point, GeodesicBearing};

#[test]
fn y_out_of_positive_range() {

    // y is out of range of Lat vals will return NaN
    let x = point!( x: 0_f64 , y: 91_f64);
    let y = point!( x: 0_f64 , y: 0_f64);

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

#[test]
fn y_out_of_negative_range() {

    // y is out of range of Lat vals will return NaN
    let x = point!( x: 0_f64 , y: -91_f64);
    let y = point!( x: 0_f64 , y: 0_f64);

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

#[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())
}
// running 3 tests
// test y_out_of_negative_range ... ok
// test y_out_of_positive_range ... ok
// test x_out_of_range ... ok

[JosiahParry]

Note that this is using geo v 0.26.0

[michaelkirk]

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.

[urschrei]

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.

[JosiahParry]

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.

[cojmeister]

Hey, can this be assigned to me?

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

Should I make a custom Err?

[cojmeister]

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.