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

Add image service layers to address #932 #933

Merged
merged 9 commits into from
Oct 27, 2022

Conversation

tsutterley
Copy link
Contributor

@tsutterley tsutterley commented Mar 3, 2022

Queries ESRI Image Service APIs and adds image layers similar to an ImageOverlay. Image Service APIs allow the creation of mosaics with options for pre-defined raster functions, interpolation strategies, band combinations, time ranges, etc.

Additional updates to projections to add EPSG:5936 (Alaska Polar Stereographic) and more EPSG:3031 (Antarctic Polar Stereographic) options

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.

Neat! Thanks!

ipyleaflet/leaflet.py Outdated Show resolved Hide resolved
js/src/layers/ImageServiceLayer.js Outdated Show resolved Hide resolved
@tsutterley tsutterley force-pushed the imageserver branch 2 times, most recently from f5d8a1b to 508837a Compare March 9, 2022 02:13
@tsutterley tsutterley force-pushed the imageserver branch 6 times, most recently from bbc108c to 7e2b068 Compare March 18, 2022 18:26
@tsutterley tsutterley force-pushed the imageserver branch 3 times, most recently from 7f1b56d to 2291070 Compare March 19, 2022 00:05
@tsutterley tsutterley marked this pull request as ready for review March 21, 2022 00:01
@tsutterley

This comment was marked as resolved.

@martinRenou
Copy link
Member

Thanks I'll try to find time to test this out and review :)

@tsutterley tsutterley changed the title feat: starting work on adding image service layers to address #932 [WIP] adding image service layers to address #932 Apr 18, 2022
@tsutterley
Copy link
Contributor Author

Ok! I think this is at a good place for review. I needed to extend from a Layer model versus a RasterLayer model and that fixed my JS-Python binding issues

@tsutterley tsutterley force-pushed the imageserver branch 2 times, most recently from a3da296 to e138882 Compare April 18, 2022 18:52
@tsutterley tsutterley changed the title [WIP] adding image service layers to address #932 Add image service layers to address #932 Apr 18, 2022
@s-m-t-c
Copy link

s-m-t-c commented Jun 1, 2022

The additional Polar projected WMS examples in CustomProjections.ipynb helped resolve an issue I had recently.

@dshean
Copy link

dshean commented Aug 24, 2022

Hi all. Wondering about status of this PR. @martinRenou, any other changes required?
This functionality will be really useful for interactive mapping applications in the polar regions (increasingly important), and having upstream support will be better than current workarounds. Thanks!

@HaudinFlorence
Copy link
Contributor

Thanks for this PR. This works well (as shown on this video produced with using the example notebook ImageService.ipynb)
image_service4 gif

Comment on lines 14 to 49
// image server url
url: '',
// response format
f: 'image',
// output image format
format: 'jpgpng',
// data type of the raster image
pixelType: 'UNKNOWN',
// pixel value or list of pixel values representing no data
noData: [],
// how to interpret no data values
noDataInterpretation: '',
// resampling process for interpolating the pixel values
interpolation: '',
// lossy quality for image compression
compressionQuality: '',
// order of bands to export for multiple band images
bandIds: [],
// time instance or extent for image
time: [],
// rules for rendering
renderingRule: {},
// rules for mosaicking
mosaicRule: {},
// image transparency
transparent: false,
// endpoint format for building the export image url
endpoint: '',
// image service attribution
attribution: '',
// coordinate reference system
crs: null,
// emit when clicked or hovered
interactive: false,
// update interval for panning
updateInterval: 200
Copy link
Contributor

Choose a reason for hiding this comment

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

This a detail point but from an homogeneity point of view, description of the attributes are only set on the python side, in the docstrings.

Copy link
Contributor

@HaudinFlorence HaudinFlorence Aug 29, 2022

Choose a reason for hiding this comment

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

I am wondering, if the layers from ImageService are also available in wmts (I think so if I am refering to their webpage https://enterprise.arcgis.com/en/image/latest/get-started/windows/what-is-an-image-service.htm) in such a way we could add them to the xyz-services tilelayers provider used in ipyleaflet? This may be an alternative way of using the layers from ImageService

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @HaudinFlorence, great question. I wanted to put this PR together to address ImageService layers that do not have layers available from other services. This was the case for digital elevation model products from the Polar Geospatial Center (PGC), such as REMA and ArcticDEM. While ArcticDEM does have a WMS url for the gray hillshade layer, it doesn't for any other layer. REMA, unfortunately, is only available through ImageService queries. One of my colleagues suggested I just try to prompt PGC to add more services, but I thought as a first step it might be easier to add the same capabilities available through the DEM web applications to ipyleaflet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This a detail point but from an homogeneity point of view, description of the attributes are only set on the python side, in the docstrings.

Great point. I removed these comments in 93394bf

@betolink
Copy link
Contributor

betolink commented Sep 1, 2022

This looks neat!! @davidbrochart can you guys take a look and consider a merge?

Copy link
Member

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

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

Thanks a lot @tsutterley!
I commented mainly to have more explanation and understand better.
It's still not clear to me if the ImageService is something only provided by ESRI, or if there are other providers.
Maybe @betolink can answer some questions too, as he knows quite a lot in this area!

"metadata": {},
"outputs": [],
"source": [
"# note that we need to use the same projection for the our layer and the map.\n",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"# note that we need to use the same projection for the our layer and the map.\n",
"# note that we need to use the same projection for the layer and the map.\n",

" return u'{0:d}\\u2013{1:d}'.format(st.tm_year,et.tm_year)\n",
"\n",
"range_dropdown1 = Dropdown(\n",
" value=u'{0:d}\\u2013{1:d}'.format(1980,1989),\n",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
" value=u'{0:d}\\u2013{1:d}'.format(1980,1989),\n",
" value=range_formatter(years[0]),\n",

"\n",
"# watch year range function widget for changes\n",
"range_dropdown1.observe(set_year_range)\n",
"m1"
Copy link
Member

Choose a reason for hiding this comment

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

No image is displayed for zoom 9 and years 1980-1989.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point. I wasn't the happiest with this example so I removed it.

"outputs": [],
"source": [
"# ArcticDEM\n",
"# note that we need to use the same projection for the our image service layer and the map.\n",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"# note that we need to use the same projection for the our image service layer and the map.\n",
"# note that we need to use the same projection for the image service layer and the map.\n",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed 👍

" \"Height Orthometric\",\n",
" \"Slope Map\"]\n",
"raster_dropdown2 = Dropdown(\n",
" value=\"Hillshade Gray\",\n",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
" value=\"Hillshade Gray\",\n",
" value=raster_functions[3],\n",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great change 👍

time: List, default []
time instance or extent for image
rendering_rule: dict, default {}
rules for rendering
Copy link
Member

Choose a reason for hiding this comment

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

What are possible values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great point. I added an enumerated list of possible values

mosaic_rule: dict, default {}
rules for mosaicking
transparent: boolean, default False
If true, the image service will return images with transparency
Copy link
Member

Choose a reason for hiding this comment

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

I tried setting transparent=True but it doesn't seem to have any effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right... this appears to be defunct. I removed the option.

crs: dict, default ipyleaflet.projections.EPSG3857
Projection used for this image service.
interactive: bool, default False
Emit when clicked or hovered
Copy link
Member

Choose a reason for hiding this comment

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

What is emitted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a handler for click events when interactive is True 👍

interactive: bool, default False
Emit when clicked or hovered
update_interval: int, default 200
Update interval for panning
Copy link
Member

Choose a reason for hiding this comment

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

What is updated? What is the unit of this value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point. I expanded the description here. This is using L.Toggle with units in ms

ipyleaflet/projections.py Show resolved Hide resolved
docs: add options to `format`, `pixel_type`, `no_data_interpretation` and `endpoint`
feat: add function for handling mouse clicks
refactor: remove columbia glacier example
remove transparency option (defunct)
Copy link
Contributor

@betolink betolink left a comment

Choose a reason for hiding this comment

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

I think the branch is ready to be merged with a minor observation on the layer naming convention. This work will allow us to use more base layers in polar projections which has been needed for a long time! @davidbrochart @dshean

ipyleaflet/projections.py Show resolved Hide resolved
ipyleaflet/projections.py Show resolved Hide resolved
examples/ImageService.ipynb Outdated Show resolved Hide resolved
examples/ImageService.ipynb Outdated Show resolved Hide resolved
@betolink
Copy link
Contributor

Is there any particular thing @tsutterley needs for this PR @davidbrochart? I see just one change request but perhaps was already addressed?

Copy link
Member

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

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

Thanks a lot @tsutterley, this looks great!
Just some minor comments, but for me this is ready to merge.
Maybe @martinRenou wants to take a look?

format: string, default "jpgpng"
format of exported image

- ``jpgpng``
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean:

Suggested change
- ``jpgpng``
- `'jpgpng'`

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

put the docstring edits in! thanks @davidbrochart

Copy link
Member

Choose a reason for hiding this comment

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

But shouldn't it be:

`'jpgpng'`

instead of:

``"jpgpng"``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

are the docstrings in ipyleaflet rendered from markdown and not restructuredtext?

Copy link
Member

Choose a reason for hiding this comment

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

Yes you're right, I think it's reStructuredText 👍

- ``U4``
- ``U8``
- ``UNKNOWN``
no_data: list, default []
Copy link
Member

Choose a reason for hiding this comment

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

Maybe:

Suggested change
no_data: list, default []
no_data: List[int], default []

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.

LGTM!

@martinRenou martinRenou merged commit aabb752 into jupyter-widgets:master Oct 27, 2022
@tsutterley tsutterley deleted the imageserver branch October 27, 2022 15:48
@martinRenou
Copy link
Member

This looks super nice thank you!

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.

7 participants