Skip to content

[WIP] Overhaul symbology schema and menus #754

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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mfisher87
Copy link
Member

@mfisher87 mfisher87 commented Jun 11, 2025

Description

The functionality we offer in the symbology menu has diverged from the schema. For example, we now offer the ability to set render type for color and radius independently, and the schema doesn't support that yet.

We also recognized that having a heatmap layer type is a mismatched abstraction; it should just be a render type. We're planning to remove that layer type in this PR as well.

TODO:

  • Implement separate schemas for color and size
  • Update symbology panel
    • Separate color and size tabs into different react components with separate state
  • Update QGIS import/export
  • Raise error when opening outdated schema (e.g. implement a global variable e.g. SUPPORTED_SCHEMA_BOUNDS = ["0.6.0", "*"] and raise error if the project file's schema version is outside the bounds)
  • Update example project files
  • Update Python API
    • create_color_expr -- rethink the API from scratch maybe? Make a Symbology class? Use generated class from the schema?
    • Remove add_heatmap_layer

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

Copy link
Contributor

Binder 👈 Launch a Binder on branch mfisher87/jupytergis/symbology-overhaul

@mfisher87 mfisher87 force-pushed the symbology-overhaul branch 6 times, most recently from 27220fa to 71b761b Compare June 30, 2025 17:46
@mfisher87 mfisher87 force-pushed the symbology-overhaul branch from 71b761b to 966064c Compare July 2, 2025 19:51
@mfisher87 mfisher87 added bug Something isn't working enhancement New feature or request labels Jul 3, 2025
@mfisher87
Copy link
Member Author

mfisher87 commented Jul 3, 2025

@martinRenou @arjxn-py @gjmooney I'd love to hear anyone's thoughts about what I'm doing in c6af37c. Read the commit message for context. It's not ideal, but I believe it's a necessary cost to using our code generation tools.

This will enable us to extract common field definitions into separate files that can be shared between schemas.

This presents a new problem, however; the TypeScript type generation dereferences schemas instead of importing from referenced schemas. So when we include references, we render both the referenced schema and also render the same schema within schemas that reference it. So when we attempt to import those referenced schemas in packages/schema/src/types.ts, we import the same name twice and produce an error. I'm not sure yet how to work around this...

Having different code generation tools which expect different things and have different output behaviors is very difficult to reason about!

@@ -10,7 +10,6 @@
},
"selectedAttribute": {
"type": "string",
"title": "Attribute",
Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to use this for form generation, but it's also affecting type generation.

mfisher87 and others added 8 commits July 11, 2025 09:14
We don't use the "state" suffix for anything else :)

Co-authored-by: martinRenou <martin.renou@gmail.com>
Co-authored-by: Kristin Davis <kristin.p.davis@gmail.com>
Co-authored-by: Kristin Davis <kristin.p.davis@gmail.com>
Co-authored-by: martinRenou <martin.renou@gmail.com>
…work with $refs

The root problem is that our TS-type-generation tool,
jsonschema-to-typescript, expects $ref paths to be relative to the
schema root directory, while our Python-type-generation tool,
datamodel-codegen, expects $ref paths to be relative to the file
containing the $ref.

To work around this, I'm adding a pre-processing step to the Python type
generation code which converts the paths to look like datamodel-codegen
expects.
@mfisher87 mfisher87 force-pushed the symbology-overhaul branch from 5875068 to 85901c5 Compare July 11, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

1 participant