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

LatLon(missing, missing) triggers a stack overflow error #47

Closed
austinlostinboston opened this issue Oct 29, 2018 · 3 comments · Fixed by #48
Closed

LatLon(missing, missing) triggers a stack overflow error #47

austinlostinboston opened this issue Oct 29, 2018 · 3 comments · Fixed by #48

Comments

@austinlostinboston
Copy link

This doesn't seem like an intended result for passing missing to LatLon. At the very least, this should trigger an internal exception within Geodesy. There also might be a greater question of whether a LatLon(missing, missing) should be allowed to exist.

julia> LatLon(missing, missing)
ERROR: StackOverflowError:
Stacktrace:
[1] LatLon(::Missing, ::Missing) at /Users/my_name/.julia/packages/Geodesy/qjth5/src/points.jl:37     (repeats 80000 times)
@visr
Copy link
Member

visr commented Oct 29, 2018

Geodesy.jl/src/points.jl

Lines 33 to 37 in 2774631

struct LatLon{T <: Number}
lat::T
lon::T
end
LatLon(lat, lon) = LatLon(promote(lat, lon)...)

Oh that's not so nice behavior.
Since missing is not a Number, it is not supported in LatLon currently. But when you try it, it calls the method with promote, which, since the result is still not two identical subtypes of Number, calls itself again and again, leading to the StackOverflowError. Same happens for LatLon('a', 'b') for instance.

I'm guessing that method should also be restricted to Number types:

LatLon(lat::Number, lon::Number) = LatLon(promote(lat, lon)...)

@andyferris
Copy link
Contributor

I'm guessing that method should also be restricted to Number types

Yes, for the short term, let's do that!

@austinlostinboston For the moment, you might have better luck considering thet LatLon as an "entire" object that may be missing, i.e. model your data with Union{Missing, LatLon{Float64}} or whatever.

@austinlostinboston
Copy link
Author

All good suggestions and thanks for the help @visr and @andyferris!

visr added a commit that referenced this issue Nov 4, 2018
now it gives a MethodError instead
fixes #47
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

Successfully merging a pull request may close this issue.

3 participants