Skip to content

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 mfisher87 force-pushed the symbology-overhaul branch from c69adb6 to 5875068 Compare July 7, 2025 16:31
@mfisher87 mfisher87 force-pushed the symbology-overhaul branch from 5875068 to 85901c5 Compare July 11, 2025 15:20
@mfisher87 mfisher87 moved this from Todo to In Progress in JupyterGIS Prioritization Jul 22, 2025
mfisher87 and others added 8 commits July 22, 2025 15:23
We don't use the "state" suffix for anything else :)

Co-authored-by: martinRenou <[email protected]>
Co-authored-by: Kristin Davis <[email protected]>
…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 85901c5 to a85db58 Compare July 22, 2025 21:23
@mfisher87
Copy link
Member Author

mfisher87 commented Jul 22, 2025

Currently, our "barrel file" packages/schema/src/types.ts imports types from the generated typescript files to make them available from one place. However, json-schema-to-typescript generates and exports duplicate types when $refs are used. A hero has submitted a PR to update json-schema-to-typescript to handle this case, and import instead of inlining referenced types! This seems to me to be the ideal solution, but it's not merged. Perhaps it's worth publishing a fork to NPM until this functionality is merged... I haven't tested it yet.

For now, the layer schema only references the graduated schema. We'll
figure out how to make them mutually exclusive later!
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: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant