Skip to content

Conversation

davidbrochart
Copy link
Collaborator

@davidbrochart davidbrochart commented Jun 6, 2025

Description

There are situations where one wants to add a layer to the shared model without saving it to disk (in the .jGIS file). These "transient" layers are not meant to exist forever, like a raster layer that would load tiles from a stable origin (e.g. OSM). For instance, they might be computed by a live kernel, and they disappear as soon as the kernel is shut down. A good example of such ephemeral layers can be generated by jupytergis-tiler.
This PR adds a "transient" attribute to a layer model, which is False by default. When set to True, the layer won't be saved to the .jGIS file.

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

📚 Documentation preview: https://jupytergis--735.org.readthedocs.build/en/735/
💡 JupyterLite preview: https://jupytergis--735.org.readthedocs.build/en/735/lite

@davidbrochart davidbrochart marked this pull request as draft June 6, 2025 13:00
Copy link
Contributor

github-actions bot commented Jun 6, 2025

Binder 👈 Launch a Binder on branch davidbrochart/jupytergis/transient-layer

@davidbrochart davidbrochart added the enhancement New feature or request label Jun 6, 2025
@davidbrochart
Copy link
Collaborator Author

TODO:

  • I think that the "layerTree" and "sources" should also be updated.
  • I'm not sure if the "transient" attribute should be defined in the model schema.

Copy link
Contributor

github-actions bot commented Jun 6, 2025

Integration tests report: appsharing.space

@mfisher87
Copy link
Member

How would this work in a collaborative scenario?

@davidbrochart
Copy link
Collaborator Author

The transient layers would be visible for every user opening the .jGIS file, even though they are not saved in the file, as long as the tiles are served by e.g. a kernel. This is because the in-memory model has the layers, they are just not dumped to disk.
There is an edge case though: if the kernel serving the tiles is shut down, and the model still lives in memory (because it's opened by a user), another user will try to load the tiles but that would fail with a 404. It's not that bad since the layer won't be displayed, and when the shared model is dropped from memory then that won't happen anymore. A best practice would be to remove the layer from the model e.g. before closing the notebook, but I don't think we can enforce that.

@SylvainCorlay
Copy link
Contributor

Thanks for opening this @davidbrochart. You know I am in favor of this!

@mfisher87
Copy link
Member

That makes sense, thanks for the explanation!

@davidbrochart
Copy link
Collaborator Author

Hmm I think this PR won't have the desired effect, because most of the time the model is loaded from the YStore (if there are no out-of-band changes), where the layers are present anyway.

@davidbrochart
Copy link
Collaborator Author

But looking at jupyter-collaboration's code, the model in the YStore is replaced with the model from the file if their contents differ, so I think this PR will indeed have the desired effect.

@davidbrochart davidbrochart marked this pull request as ready for review June 10, 2025 09:24
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.

Thanks!

I have some questions/suggestions:

  • do we really need to make both the source and layer transient? If the source is transient, it's probably enough (we should automatically consider the layers pointing to that source as transient)
  • we should also change the JSON schemas for the sources to include that new transient parameter
  • (maybe for later PR) we should add a small icon on the front-end in the layers tree view for transient layers

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.

4 participants