Closed gaelforget closed 2 months ago
Hi @asinghvi17 and @haakon-e , I am hoping you could give this PR a look while the topic is fresh in our minds. Should have time to iterate upon review next week if PR cannot be merged as is. Thanks for everything
ohh... this is a good use case. If will be great to have it.
Quick bump @gaelforget
ohh... this is a good use case. If will be great to have it.
do you mean adding a plot like the one below in the docs of GeoMakie?
https://juliaocean.github.io/OceanRobots.jl/dev/examples/OceanOPS.html
I should be able to do that in separate PR if you don't mind me adding OceanRobots.jl in docs/Project.toml
all set with suggested revision I think
Will review tomorrow, it's getting a bit late here unfortunately. Thanks for the update!
Will review tomorrow, it's getting a bit late here unfortunately. Thanks for the update!
Awesome! Thank you in advance
Do you particularly care about the name here? I'm thinking of upstreaming this to GeometryOps, maybe in a method called cut_antimeridian
. That also gets rid of the "piracy" (relatively benign, but not that great nevertheless).
This would allow the operation to be generalized to polygons (which need to be closed) and so on and so forth. Currently, the algorithm does not inject points at the line segment that is split and is probably not robust to floating point error - that's something that moving it to GeometryOps (or at least a GeometryOps-like workflow) can also fix.
We can merge the PR with a couple of changes that I will push for now, but I would like to change the behaviour later, perhaps as a v0.8.0 after Juliacon.
Do you particularly care about the name here? I'm thinking of upstreaming this to GeometryOps, maybe in a method called
cut_antimeridian
. That also gets rid of the "piracy" (relatively benign, but not that great nevertheless).
Name : seems to me that split
with a keyword for lon
, lon0
, or longitude
+ a reference to antemeridian
in the docstring would be nicer, and makes sense since this operation is a direct analogy to what Base.split
does. Also cut_antimeridian
is kind of confusing (cut_at_antimeridian
?) and long.
Piracy: not sure what's the issue here, or in what way this is piracy
, aside from the fact that split
is a common word that applies to strings amongst many other things.
Upstreaming this to GeometryOps : seems reasonable, happy to send a PR there later is that helps, but will need to look at GeometryOps first
This would allow the operation to be generalized to polygons (which need to be closed) and so on and so forth.
Maybe I am confused but isn't it that objects in geojson and shape files are typically cut at antimeridian=+-180 (i.e., they are not closed polygons in the first place) which is why they plot nicely when lon0=0 ?
One way to think about generalizing this would be split(x,polygon=p)
where p defines a closed region on the surface of the Earth or on the projection plane (e.g. the GeoAxis). But there are probably several ways to look at this, e.g. depending if one takes geo
to mean geometry, geography, or geospatial.
Currently, the algorithm does not inject points at the line segment that is split and is probably not robust to floating point error - that's something that moving it to GeometryOps (or at least a GeometryOps-like workflow) can also fix.
I don't know how GeometryOps does things or how you'd inject points, but from discussion in https://github.com/MakieOrg/GeoMakie.jl/pull/128 with @haakon-e I was under the impression that adding points might complicate zooming in and out interactively with GLMakie, and that it might prevent reversibility (e.g. changing lon0 from 0 to -160 and back to 0 gradually via an observable lon0 you'd end up with a bunch more points at the end or something like that)
We can merge the PR with a couple of changes that I will push for now, but I would like to change the behaviour later, perhaps as a v0.8.0 after Juliacon.
Sounds good
Following up on https://github.com/MakieOrg/GeoMakie.jl/pull/128
split
method from @haakon-eResulting plot should look as shown below.
ps. revised code should readily work with https://github.com/MakieOrg/GeoMakie.jl/pull/207 , should be simple to merge (I tried)