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

Vector symbology api #163

Merged
merged 14 commits into from
Oct 1, 2024
Merged

Conversation

gjmooney
Copy link
Collaborator

@gjmooney gjmooney commented Sep 28, 2024

Changes geojson notebook API to use color expressions. Also loads colors for vector tile layers from qgz files, and exports color information when exporting to qgz.

Copy link
Contributor

Binder 👈 Launch a Binder on branch gjmooney/jupytergis/vector_symbology_api

@gjmooney gjmooney added the enhancement New feature or request label Sep 30, 2024
@gjmooney gjmooney marked this pull request as ready for review October 1, 2024 09:16
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.

Neat!!

I just have a single comment/question, otherwise I think it's good to merge

# TODO Load source-layer properly, from qgis symbology?
try:
source_layer = get_source_layer_names(url)[0]
layer_parameters["sourceLayer"] = source_layer
except ValueError:
pass
# TODO Load style properly

layer_parameters.update(type="fill")
Copy link
Member

Choose a reason for hiding this comment

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

Should we keep this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm pretty sure the source_layer stuff isn't being used anywhere right now, so it should be fine to get rid of. The type is required in the schema so we should leave that for now.

Copy link
Member

Choose a reason for hiding this comment

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

Ok! Thanks for your answer

Let's merge this then!

@martinRenou martinRenou merged commit 0fbf57e into geojupyter:main Oct 1, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants