feat: Table of contents

[ATL2001]

This isn't yet a fully working solution, but figured it would be good to put it out there to get feedback on what it does so far

I've resurrected the old code I wrote to make the TOC, and fixed it up to work with the new linting rules.

I've added two new public functions to lonboard.controls make_toc and make_toc_with_settings (I'm very open to renaming those if you have better ideas) and a lot of other private helper functions that users won't need.

• The make_toc function will make a simple table of contents that can control layer visibility

• The make_toc_with_settings function will make a table of contents which can control the layer visibility, and also has a button, which when pressed, will allow changes to be made to the layer's properties. The layer traits that are None when the TOC is created are not included in the settings because I don't know of a way to make the ipywidgets widgets work with None (I don't believe it's possible, but would welcome being wrong about that)

I've also added a new title trait to BaseLayer so the layer's title can be shown in the TOC which is defaulted to 'layer'

Additionally, I set the default value on BaseLayer.highlight_color to be [0, 0, 128, 128] to match what is in the docstring for the default value. (when the make_toc_with_settings function was trying to access the trait without the default it was throwing an error)

what's not working:

The current implementation blows up when you create a TOC with settings and a layer uses a continuous colormap (like what is in the examples/map_challenge 6-asia notebook) (and I assume the same will happen for a categorical colormap) I haven't thought much about how to handle those situations yet, but if you have any ideas, I'm all ears.

[kylebarron]

This isn't yet a fully working solution, but figured it would be good to put it out there to get feedback on what it does so far

Thanks! I'd like to be more attentive to PRs than before.

I've added two new public functions to lonboard.controls make_toc and make_toc_with_settings (I'm very open to renaming those if you have better ideas) and a lot of other private helper functions that users won't need.

Perhaps we could make this a method on the Map object itself? Like Map.toc()?

It's a code smell to me to have both make_toc and make_toc_with_settings. Can we have an optional, default parameter to make_toc, so the latter is just make_toc(settings=...).

I think we should also brainstorm more on what this should be called. Instead of TOC maybe Layer Widget, Layer Selector, Legend, Layer Control, etc.

Widget and Control are becoming confusing terminology. A lot of JS web maps like Mapbox GL JS called "Control" the extra map elements, like distance/compass. Deck.gl calls those Widgets.

While in Python, ipywidgets-based widgets have the opposite meaning of "widget". In particular, what do we call something appearing on the map itself (from JS) vs something separately in the notebook page that communicates with the map element. We should solidify this terminology.

• The make_toc function will make a simple table of contents that can control layer visibility
• The make_toc_with_settings function will make a table of contents which can control the layer visibility, and also has a button, which when pressed, will allow changes to be made to the layer's properties. The layer traits that are None when the TOC is created are not included in the settings because I don't know of a way to make the ipywidgets widgets work with None (I don't believe it's possible, but would welcome being wrong about that)

I've also added a new title trait to BaseLayer so the layer's title can be shown in the TOC which is defaulted to 'layer'

We should decide whether a layer's title is a property of the map or of the layer itself.

In particular, a layer can be rendered in multiple maps. Should the layer always have the same name? It's more flexible to allow each instance of the layer to have a different name. In that case, it wouldn't be an attribute of the layer but instead the Map object would hold essentially a dict of layer instances, keyed by name.

Additionally, we should default the title of a layer to the name of the class. So the default title for a polygon layer would be "PolygonLayer".

Additionally, I set the default value on BaseLayer.highlight_color to be [0, 0, 128, 128] to match what is in the docstring for the default value. (when the make_toc_with_settings function was trying to access the trait without the default it was throwing an error)

See inline note.

what's not working:

The current implementation blows up when you create a TOC with settings and a layer uses a continuous colormap (like what is in the examples/map_challenge 6-asia notebook) (and I assume the same will happen for a categorical colormap) I haven't thought much about how to handle those situations yet, but if you have any ideas, I'm all ears.

Perhaps we should remove color options from this PR and work on that in a follow up PR? I figure that most of the time a user will add some sort of colormap? Or, if a layer is using a custom colormap (anything other than a scalar color), then we can just print "custom" in the legend for that layer.

[ATL2001]

In the original code I wrote I had one function and it had a parameter for with_settings, a boolean which allowed the settings to be toggled on or off but ruff complained about it so i just made it into two functions (https://docs.astral.sh/ruff/rules/boolean-type-hint-positional-argument) (I personally don't think I like that linting rule very much😄) I'd be happy to consolidate it back and drop a NOQA FBT001 on it.

I don't have any strong feeling either way on the name of it, I do like layer control, but your idea of why to not use control is valid.

In my experiences making map products and geospatial visuals I've not needed to make one layer and use it in different maps with a different name unless I also needed to change other parts of the layer as well (colors, line widths, popups something like that), so in that case you'd need to make a new layer anyway.

default title, that's a good idea!

I think I like the idea of "custom" or something along those lines for the colors that aren't just one color (I'd rather not throw that part out totally for now, because what I have right now works great for single colors),

I'll try to carve out some time this weekend and see what I can come up with.

[ATL2001]

I've got a .layer_control() method on the map itself now, and I believe I've got the color stuff sorted out with adding a label to the settings control that just says "Custom" instead of making a widget to control the value for the traits who's values are array like.

And I realized I wasn't understanding properly what ruff was telling me about the boolean in the function, it just wanted me to make it a non-positional argument, so now there's just one method on the map to make the layer control with an argument for include settings, no rule ignoring needed 😄

[kylebarron]

but ruff complained about it

Ruff only complains about positional boolean arguments. If you make it a keyword-only argument then ruff won't complain.

I think I like the idea of "custom" or something along those lines for the colors that aren't just one color (I'd rather not throw that part out totally for now, because what I have right now works great for single colors),

👍

[ATL2001]

Ruff only complains about positional boolean arguments. If you make it a keyword-only argument then ruff won't complain.

yeah, once I reread it and looked at the example, it made a lot more sense! that's how I've got it now 😎

[kylebarron]

@ATL2001 did you plan more changes here before asking for my review? Or is this ready for an in-depth review? Should we try to get it in for the next release?

[ATL2001]

I actually do want to make one more small tweak to the widget I have linked color accessor (I'd like to add another widget to control the alpha similar to how it works in the panel app demo) I'll see if I can wire it up tonight

[ATL2001]

I've still got a kink to work out, it'll likely be the weekend before I can iron it out 😞

[kylebarron]

No rush, we can make another release then. I think I want to release #846 soon, and along with the python 3.9 deprecation it needs a minor release. In contrast to Rust, it's relatively low-downside to making minor releases, so we can make more releases soon in the future.

[ATL2001]

ok, ready for review now! 👍