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

Make Leaflet-Geoman ignore created layers by default #1220

Conversation

riccardoporreca
Copy link
Contributor

@riccardoporreca riccardoporreca commented Jul 22, 2024

Fixes #1215, see discussion therein.

The possible usage of pm_ignore=False is now also included in the in 'Geoman Draw Control' documentation page (with some minor adjustments to existing content)

@riccardoporreca
Copy link
Contributor Author

Let me know if and how I should add an entry in CHANGELOG.md

@riccardoporreca
Copy link
Contributor Author

The docs failure is unrelated to the changes, see https://app.readthedocs.org/projects/ipyleaflet/builds/25076525/#238126839--263

The geopandas.dataset has been deprecated and was removed in GeoPandas 1.0. You can get the original 'naturalearth_lowres' data from https://www.naturalearthdata.com/downloads/110m-cultural-vectors/.

I could build the docs locally commenting out the corresponding example, to verify how the updated Geoman Draw Control page looks.

Copy link
Contributor

@iisakkirotko iisakkirotko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was even simpler than I thought, no comments there. I tried it out and everything seems to work as expected.

There seem to be some commits that are unrelated to the PR, but I'll let @martinRenou be the judges of whether that is acceptable

Comment on lines +2 to +4
===================

``GeomanDrawControl`` allows one to draw various shapes on the map.
``GeomanDrawControl`` allows one to draw various shapes on the map.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two lines in seem unrelated to the commit they are in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, these two minor changes came when I rendered the fix to the list locally, since my IDE drops trailing spaces and there was a warning in Sphinx about the nr of = that I took the chance of fixing.

I should probably have at least added an additional line in the commit message stating stg like "Also fix some minor cosmetic RST formatting."

If you prefer to have a more accurate commit history, I am happy to force-push them as individual changes. Let me know, no big deal for me.

@riccardoporreca
Copy link
Contributor Author

@iisakkirotko, @martinRenou, any plan to go back to this?

Meanwhile, I am happy to share the interim solution we have been adopting while waiting for this to be part of a new release of ipyleaflet, in case it is helpful to others.

import ipyleaflet
from ipyleaflet import Bool


class GeomanIgnored(ipyleaflet.Layer):
    pm_ignore = Bool(True).tag(sync=True, o=True)


class PolygonGeomanIgnored(ipyleaflet.Polygon, GeomanIgnored):
    pass


class CircleGeomanIgnored(ipyleaflet.Circle, GeomanIgnored):
    pass


class MarkerGeomanIgnored(ipyleaflet.Marker, GeomanIgnored):
    pass


...

@iisakkirotko
Copy link
Contributor

iisakkirotko commented Dec 3, 2024

Hi @riccardoporreca! Other than the comment I already left, the PR looks good to me. @martinRenou if that doesn't bother you, as far as I'm concerned, feel free to merge.

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@martinRenou martinRenou merged commit 7f7343d into jupyter-widgets:master Dec 5, 2024
1 check failed
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.

GeomanDrawControl can modify arbitrary shapes on the map
3 participants