georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
143 stars 46 forks source link

Update to PROJ 8.1.0 #89

Closed urschrei closed 3 years ago

urschrei commented 3 years ago

Merging is blocked on https://github.com/georust/docker-images/pull/13

michaelkirk commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

urschrei commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

urschrei commented 3 years ago

Tests were never gonna pass on the outdated images, were they?

urschrei commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

urschrei commented 3 years ago

Now we're getting somewhere (why are those tests failing on ubuntu?)

michaelkirk commented 3 years ago

Now we're getting somewhere (why are those tests failing on ubuntu?)

FYI, I've just reproduced this failure locally:

$ docker run -v /Users/mkirk/src/georust/docker-images/rust-1.51/../..:/tmp/georust -w /tmp/georust/proj docker.io/georust/proj-ci:rust-1.51 cargo test --no-default-features

failures:                                                                                                                                                                     

---- proj::test::test_conversion stdout ----                                                                                                                                  
thread 'proj::test::test_conversion' panicked at 'assert_relative_eq!(t.x(), 1450880.2910605017)                                                                              

    left  = 1450880.2910605012                                                                                                                                                
    right = 1450880.2910605017                                                                                                                                                

', src/proj.rs:1103:9                                                                                                                                                         
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- proj::test::test_inverse_projection stdout ----
thread 'proj::test::test_inverse_projection' panicked at 'assert_relative_eq!(t.x(), 0.43633200013698786)

    left  = NaN
    right = 0.43633200013698786

', src/proj.rs:1066:9

---- proj::test::test_london_inverse stdout ----
thread 'proj::test::test_london_inverse' panicked at 'assert_relative_eq!(t.x(), 0.0023755864830313977)

    left  = NaN
    right = 0.0023755864830313977

', src/proj.rs:1083:9

---- proj::test::test_projection stdout ----
thread 'proj::test::test_projection' panicked at 'assert_relative_eq!(t.x(), 500119.7035366755, epsilon = 1e-5)

    left  = NaN
    right = 500119.7035366755

', src/proj.rs:1051:9

failures:
    proj::test::test_conversion
    proj::test::test_inverse_projection
    proj::test::test_london_inverse
    proj::test::test_projection

test result: FAILED. 11 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.48s

error: test failed, to rerun pass '--lib'
michaelkirk commented 3 years ago

... and interestingly with proj-ci:rust-1.53, I get only 1 of the same 4 failures as with proj-ci:rust-1.51...

$ docker run -v /Users/mkirk/src/georust/docker-images/rust-1.53/../..:/tmp/georust -w /tmp/georust/proj docker.io/georust/proj-ci:rust-1.53 cargo test --no-default-features
---- proj::test::test_conversion stdout ----
thread 'proj::test::test_conversion' panicked at 'assert_relative_eq!(t.x(), 1450880.2910605017)

    left  = 1450880.2910605012
    right = 1450880.2910605017

', src/proj.rs:1103:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    proj::test::test_conversion

test result: FAILED. 14 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.46s

error: test failed, to rerun pass '--lib'
urschrei commented 3 years ago

bors try

urschrei commented 3 years ago

I have no idea why we were seeing NaN errors on 1.51, and that is somewhat concerning, but: I've fixed the "actual" error we were seeing by using a known-good string for the NAD83 (feet) to NAD83 (meters) conversion by feeding PROJ the EPSG codes, then generating the pipeline string. I don't know where the original string came from (mea culpa), but the source of the replacement is now noted inline.

Test failures due to different values when we update to a newer PROJ version aren't concerning (to me) if they're small (the seventh decimal place is 1.11 cm at the equator) since the values used in the EPSG db used by PROJ are updated periodically, so long as we're using known-to-be-correct inputs – the alternative is playing epistemological whack-a-mole with at least two unknowns, and nobody's got time for that.

bors[bot] commented 3 years ago

try

Build succeeded:

urschrei commented 3 years ago

bors r=michaelkirk

bors[bot] commented 3 years ago

Build succeeded: