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.

Sign up for GitHub

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

Jump to bottom

wxGUI AddWSDialog: Fix render WMTS layer with OGC:CRS84 coor system #674

Merged
merged 1 commit into from
Jun 4, 2020
Merged

wxGUI AddWSDialog: Fix render WMTS layer with OGC:CRS84 coor system #674

merged 1 commit into from
Jun 4, 2020

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented May 26, 2020 •
edited
Loading

To reproduce:

  1. Open wxGUI Add web service layer dialog
  2. From default profiles choose NASA GIBS WMTS item
  3. Select some layer from the list
  4. Choose source projection

Default behavior:

add_ws_dialog_wmts_src_proj_def

Missing source projection (Chooice widget items) for every layer in the list

Expected behavior:

add_ws_dialog_wmts_src_proj_exp

Chooice source projection widget has correct source projection item OGC:CRS84

@neteler
Copy link
Member

neteler commented May 27, 2020

Thanks for your fix.

I just saw (maybe it can be ignored), that the change fails on Ubuntu16.04:

/usr/bin/install -c  -m 644 wms_base.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_base.py
/usr/bin/install -c  -m 644 wms_drv.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_drv.py
/usr/bin/install -c  -m 644 wms_gdal_drv.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_gdal_drv.py
/usr/bin/install -c  -m 644 wms_cap_parsers.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_cap_parsers.py
/usr/bin/install -c  -m 644 srs.py /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/srs.py
python3 -t -m py_compile /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_base.py
  File "/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_base.py", line 695
    return f"OGC:CRS{srs}"
                         ^
SyntaxError: invalid syntax

make[3]: *** [/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_base.pyc] Error 1
../../include/Make/Python.make:5: recipe for target '/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/etc/r.in.wms/wms_base.pyc' failed
make[3]: Leaving directory '/home/runner/work/grass/grass/scripts/r.in.wms'

@petrasovaa
Copy link
Contributor

The f strings are coming with Python 3.6, Ubuntu 16 has Python 3.5. So far we have been using 3.5 as minimum version. We could discuss changing it to 3.6 (on dev list perhaps), for grass 8 we could definitely go with 3.6, but maybe not for 7.8 series.

So @tmszi please change it to python 3.5 compatible syntax, thank you!

@tmszi tmszi force-pushed the fix_add_ws_lyr_dialog_wmts_ogc_crs84 branch from 4fb4102 to 00ca4c8 Compare May 27, 2020 13:41
@tmszi
Copy link
Member Author

tmszi commented May 27, 2020

You're welcome. All right, I fixed it.

@petrasovaa petrasovaa self-requested a review May 30, 2020 03:08
@petrasovaa petrasovaa added bug Something isn't working GUI wxGUI related labels May 30, 2020
@petrasovaa
Copy link
Contributor

The layer is still not showed, I suspect that's a different problem, but could you please confirm that it's unrelated to this? Also could you please provide little bit more context to this fix?

petrasovaa
petrasovaa reviewed May 31, 2020
View reviewed changes
gui/wxpython/web_services/widgets.py Outdated
@@ -709,7 +710,13 @@ def CreateCmd(self):

if 'srs' not in self.drv_props['ignored_params']:
i_srs = self.params['srs'].GetSelection()
epsg_num = int(self.projs_list[i_srs].split(':')[-1])
try:
int(self.projs_list[i_srs].split(':')[-1])
Copy link
Contributor

Choose a reason for hiding this comment

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

This line seems to be redundant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it is. All right, I fixed it.

@tmszi tmszi force-pushed the fix_add_ws_lyr_dialog_wmts_ogc_crs84 branch from 00ca4c8 to 92f67a2 Compare May 31, 2020 05:23
@tmszi
Copy link
Member Author

tmszi commented May 31, 2020 •
edited
Loading

The layer is still not showed, I suspect that's a different problem, but could you please confirm that it's unrelated to this? Also could you please provide little bit more context to this fix?

I try test render NASA Columnar Cloud Liquid Water (Night, AMSR2, GCOM-W1) layer. Did you get an error message?

render_ws_nasa_layer

NASA Surface Rain Rate (Day, AMSR2, GCOM-W1) layer:

render_ws_nasa_layer

@tmszi tmszi force-pushed the fix_add_ws_lyr_dialog_wmts_ogc_crs84 branch from 5a230ff to 92f67a2 Compare May 31, 2020 09:14
@petrasovaa
Copy link
Contributor

The layer is still not showed, I suspect that's a different problem, but could you please confirm that it's unrelated to this? Also could you please provide little bit more context to this fix?

I try test render NASA Columnar Cloud Liquid Water (Night, AMSR2, GCOM-W1) layer. Did you get an error message?

Hm, can't reproduce it, I am getting the image now.

petrasovaa
petrasovaa reviewed Jun 1, 2020
View reviewed changes
gui/wxpython/web_services/widgets.py Outdated
@@ -709,7 +710,9 @@ def CreateCmd(self):

if 'srs' not in self.drv_props['ignored_params']:
i_srs = self.params['srs'].GetSelection()
epsg_num = int(self.projs_list[i_srs].split(':')[-1])
srs = self.projs_list[i_srs].split(':')[-1]
epsg_num = int(''.join(re.findall('\d+', srs)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully last small request, could you please use raw string for the expression? Quoting from Python doc:

This is complicated and hard to understand, so it’s highly recommended that you use raw strings for all but the simplest expressions.

Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes of course, I fixed it.

@tmszi tmszi force-pushed the fix_add_ws_lyr_dialog_wmts_ogc_crs84 branch from 92f67a2 to a3c68e9 Compare June 1, 2020 04:29
@petrasovaa petrasovaa merged commit dbf903e into OSGeo:master Jun 4, 2020
petrasovaa pushed a commit that referenced this pull request Jun 4, 2020
@tmszi @petrasovaa
wxGUI AddWSDialog: fix render WMTS layer with OGC:CRS84 coor system (#…
8c28dad 
…674)
@tmszi tmszi deleted the fix_add_ws_lyr_dialog_wmts_ogc_crs84 branch July 28, 2020 07:09
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrasovaa petrasovaa petrasovaa left review comments

Assignees
No one assigned
Labels
bug Something isn't working GUI wxGUI related
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tmszi @neteler @petrasovaa