search

JuliaGeo / Proj.jl

Julia wrapper for the PROJ cartographic projections library
MIT License
48 stars 9 forks source link

Proj.identify #92

Closed alex-s-gardner closed 1 year ago

alex-s-gardner commented 1 year ago

This addresses the first part of #88

alex-s-gardner commented 1 year ago

FYI: It appears that failed tests are unrelated to this PR

alex-s-gardner commented 1 year ago

I don't think libproj.jl is the right place to put Proj.identify. I think it's autogenerated. Looking through the codebase I don't see a great place to put it. Should we create a new file utilities.jl and put Proj.identify in there?

visr commented 1 year ago

Indeed libproj.jl is generated so it cannot go there. I'd say the existing crs.jl is a fine place.

Regarding the failing tests, indeed it is a known issue, see #87.

alex-s-gardner commented 1 year ago

@visr I've adopted all of your suggestions, thanks. One question I have is should we always return a vector of NamedTuples even if there is no crs found or only a single crs found? My thought was that the output should be a consistent type so I always return a vector.

visr commented 1 year ago

One question I have is should we always return a vector of NamedTuples even if there is no crs found or only a single crs found?

Yeah I agree.

I pushed a few final things in e87b0d15acdceb95aeb85a0f9dd6a53f6e9f7efd. I went back to freeing the confidence, since that is what the docs tell us, and the CRS pointers, we still use and pass back to the user. Still I couldn't trigger a segfault or anything accessing the original confidence values, so not sure if it is being cleaned up properly.