-
Notifications
You must be signed in to change notification settings - Fork 51
Add GeoPackage data format #709
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
base: main
Are you sure you want to change the base?
Add GeoPackage data format #709
Conversation
# Conflicts: # packages/base/src/constants.ts # packages/base/src/toolbar/widget.tsx # python/jupytergis_lab/jupytergis_lab/notebook/objects/__init__.py # yarn.lock
Integration tests report: appsharing.space |
packages/base/src/tools.ts
Outdated
|
||
const tables = gpkg.getFeatureTables(); | ||
const features: GeoJSON.Feature[] = []; | ||
for (const tableName of tables) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, the user should get a chance to choose which table(s) they want to load. Maybe that could be a future PR though :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users are now able to choose which tables to load
packages/base/src/tools.ts
Outdated
for (const tableName of tables) { | ||
const dao = gpkg.getFeatureDao(tableName); | ||
const bbox = dao.getBoundingBox(); | ||
const iter = gpkg.queryForGeoJSONFeaturesInTable(tableName, bbox); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how I feel about converting the data to GeoJSON. Have you looked in to https://github.com/richard-thomas/ol-load-geopackage ? (I'm not sure how it works under the hood, maybe it loads from GeoJSON too 😆 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we are using the library you suggested for vector layers, but for raster layers we're still using ngageoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we extend the jupyterlab webpack config? https://jupyterlab.readthedocs.io/en/stable/extension/extension_dev.html#custom-webpack-config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the webpack config path relative in package.json as described in the link you provided, however I'm not sure if that was what you suggested.
Hi , I am looking into this pr now , before I start , have you some local change that are not on the current pr ? |
For instance solving current merge conflict etc. |
@Gauss-Taylor-Euler It’s still WIP😊 |
Ok I see no problem, what are the thing left to do , except for merge conflict ? |
It only works for vector layers, I will add support for raster layers as well |
Ok I see , thank you. |
for more information, see https://pre-commit.ci
Some notes;
|
Description
Added support for GeoPackage data format, both to UI and to Python API, using the library geopackage-js.
I added a test for Python API but didn't add a UI test. I also did not add support for exporting GeoPackage layers to QGIS. If asked, I can do these either in this PR or another PR.
Checklist
Resolves #XXX
.Failing lint checks can be resolved with:
pre-commit run --all-files
jlpm run lint
📚 Documentation preview: https://jupytergis--709.org.readthedocs.build/en/709/
💡 JupyterLite preview: https://jupytergis--709.org.readthedocs.build/en/709/lite