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

Embed GeoJSON schema in the project to improve build reliability #165

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arjxn-py
Copy link
Contributor

Should fix #125

Copy link
Contributor

Binder 👈 Launch a Binder on branch arjxn-py/jupytergis/geojson-schema

@martinRenou
Copy link
Member

martinRenou commented Sep 30, 2024

Thanks for opening a PR! Although I kind of disagree with the fix. EDIT: I disagree because we're embedding code that is not ours here, it would be prone to error if it gets edited by mistake. We should rely on the official implementation and not copy paste it in our project.

I'd be more in favor of an approach to:

  • Download the schema once
  • Store it somewhere that is not cleaned with jlpm run clean and not tracked by git
  • Don't download it again if it's already there

That way you need an internet connection for the first build, but you don't need it anymore for subsequent builds.

@arjxn-py
Copy link
Contributor Author

arjxn-py commented Sep 30, 2024

Thanks @martinRenou, I agree with your idea and just want to confirm a couple of things that -

  • Do we want to download the schema once while building the extension (or while loading geojson layer) if base schema is not already there?
  • Would this implementation be something similar like thumbnails for raster layers thing?

@martinRenou
Copy link
Member

Do we want to download the schema once while building the extension (or while loading geojson layer) if base schema is not already there?

Once when building the application (we need the schemas to generate some Python interfaces as of today).

Would this implementation be something similar like thumbnails for raster layers thing?

Yes that could be similar 👍🏽

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.

GeoJSON schema
2 participants