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

feature: Plot recipe for Plots.jl #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

krsteffen
Copy link

Good morning, I've written a simple plot recipe for STR trees using the JuliaPlots RecipesBase package. Once you import Plots into the REPL, plotting works right away! See below for two examples.

Is this something you are interested in? I'm happy to make any changes if need be.

using Plots, Shapefile, SortTileRecursiveTree

N = 42
x, y = rand(N), rand(N);
tree = STRtree(Shapefile.Point.(x, y))
plot(tree, aspect_ratio = :equal)
scatter!(x, y, markerstrokewidth = 0)
nothing

Another example; I downloaded a small shape file from https://public.opendatasoft.com/explore/dataset/world-administrative-boundaries/export/, but please feel free to use whichever shapefile you have on hand.

using ZipFile
path = "/path/to/your/shapefile"
table = Shapefile.Table(path)
tree = STRtree(table.geometry)
plot(tree, aspect_ratio = :equal)
plot!(table.geometry, seriestype = :path, color = :black)
nothing

@maxfreu
Copy link
Owner

maxfreu commented May 13, 2024

Hi Kyle! Thanks for your contribution! I suggest a few changes / simplifications:

  1. Functions modifying their argument should have the modified arg first. https://docs.julialang.org/en/v1/manual/style-guide/#Write-functions-with-argument-ordering-similar-to-Julia-Base

  2. Instead of collecting all the x and y values I'd write sth like:

# pseudocode, didn't look everything up, nothing tested!
function collect_extents(tree::STRtree)
  extents = [(tree.rootnode.extent, 1)]  # extent, level
  for child_node in tree.rootnode.children
    collect_extents!(extents, child_node, 2)
  end
end

function collect_extents!(extents, node::SortTileRecursiveTree.STRNode, level)
  push!(extents, (node.extent, level))
  for child_node in node.children
    collect_extents!(extents, child_node, level+1)
  end
end

# similar for leaf node

function extent2xy(ex)
  # convert extent to x and y lists or tuples here
end

RecipesBase.@recipe function f(...)
  plotdata = collect_extents(tree)
  for (ex, level) in plotdata
    # unpack extent here and plot stuff
  end
end
  1. You could also provide a bool flag for reverse order plotting. Then just do for (ex, level) in plot_reverse ? reverse(plotdata) : plotdata. That way the user can decide which box is the topmost.

@krsteffen
Copy link
Author

Thank you for your feedback Max! #1–3 all sound great to me. I'll likely need to wait until the weekend to work on this code, but I will implement these changes & get back to you!

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 this pull request may close these issues.

2 participants