wxGUI loc wizard: Improve new location's CRS method selection page #721

lindakarlovska · 2020-06-19T10:36:12Z

In the third commit I have a issue after confirmation of WKT Summary page:

Still wants to open a WKT file but I am not able to figure out where is the core of this problem.

…J.4 page. Now I have a coding problem with passing wkt string as a stdin into the g.proj function.

petrasovaa · 2020-06-19T12:58:19Z

Please see https://trac.osgeo.org/grass/wiki/Submitting/General#Commitmessages how to write commit messages. If you need help, please write it in the PR discussion.

petrasovaa · 2020-06-19T13:10:50Z

gui/wxpython/location_wizard/wizard.py


        # layout
        self.sizer.SetVGap(10)
-        self.sizer.Add(StaticText(parent=self, label=_("Simple methods:")),
+        self.sizer.Add(StaticText(parent=self, label=_("Select Coordinate Reference System (CRS):")),


I was suggestion dropping "simple methods" altogether and using "Select Coordinate Reference System (CRS)" as the title of the page.

petrasovaa · 2020-06-19T13:15:46Z

gui/wxpython/location_wizard/wizard.py

        self.sizer.Add(self.radioXy,
+                       flag=wx.ALIGN_LEFT, pos=(4, 1))
+        self.sizer.Add(StaticText(parent=self, label=_("Advanced additional methods:")),


Use "Additional methods", definitely not Advanced additional methods.

petrasovaa · 2020-06-19T13:24:16Z

gui/wxpython/location_wizard/wizard.py

        self.Bind(wiz.EVT_WIZARD_PAGE_CHANGING, self.OnPageChanging)
        self.Bind(wiz.EVT_WIZARD_PAGE_CHANGED, self.OnEnterPage)

    def OnEnterPage(self, event):
-        if len(self.wktfile) == 0:
+        if len(self.customstring) == 0:


name it something like wktstring

petrasovaa · 2020-06-19T13:24:30Z

gui/wxpython/location_wizard/wizard.py

            # disable 'next' button by default
            wx.FindWindowById(wx.ID_FORWARD).Enable(False)
        else:
            wx.FindWindowById(wx.ID_FORWARD).Enable(True)

        event.Skip()
-
+        


WHITESPACE!

petrasovaa · 2020-06-19T13:27:56Z

gui/wxpython/location_wizard/wizard.py

-        else:
-            if nextButton.IsEnabled():
-                nextButton.Enable(False)
+            # check for datum tranforms


To keep things consistent with the rest, the WKTPage doesn't change much (except for the layout). g.proj is called in Summary page:

elif coordsys == 'wkt': ret, projlabel, err = RunCommand( 'g.proj', flags='jft', wkt=self.parent.wktpage.wktfile, **extra_opts)

so you need to adjust the code here. So OnPageChanging should stay similar to what was there before. The RunCommand in Summary page should look something like this:

elif coordsys == 'wkt': ret, projlabel, err = RunCommand( 'g.proj', flags='jft', wkt="-", stdin=self.parent.wktpage.wktstring, **extra_opts)

(not tested)

… of the page, used 'Additional methods' instead of 'Advanced additional methods', renamed 'customstring' to 'wktstring', removed a whitespace, changed run command of wkt selection in the summary page.

petrasovaa

I am getting error when trying to create the location with wkt (after summary page):

Traceback (most recent call last):
  File "/home/anna/dev/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/gis_set.py", line 537, in OnWizard
    grassdatabase=self.tgisdbase.GetValue())
  File "/home/anna/dev/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/location_wizard/wizard.py", line 2396, in __init__
    msg = self.OnWizFinished()
  File "/home/anna/dev/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/location_wizard/wizard.py", line 2640, in OnWizFinished
    if not self.wktpage.wktfile or \
AttributeError: 'WKTPage' object has no attribute 'wktfile'

petrasovaa · 2020-06-20T18:18:30Z

gui/wxpython/location_wizard/wizard.py

@@ -2259,7 +2237,7 @@ def OnEnterPage(self, event):
            label = 'matches file %s' % self.parent.filepage.georeffile

        elif coordsys == 'wkt':
-            label = 'matches file %s' % self.parent.wktpage.wktfile
+            label = 'matches string %s' % self.parent.wktpage.wktstring


Maybe "matches WKT string" would be better.
More importantly, the string is long, so you need to change the widget to enable scrolling, it should be similar to panelProj4string in SummaryPage.

…ile' by 'wktstring'

petrasovaa · 2020-06-21T02:49:45Z

gui/wxpython/location_wizard/wizard.py

-    def OnText(self, event):
-        """File changed"""
-        self.wktfile = event.GetString()
+    def GetWktstring(self, event):


This should still be called OnText, since it's event handler. If you name a function GetWKTString, I would expect it to return a string, which is not the case here.

petrasovaa · 2020-06-21T02:52:17Z

gui/wxpython/location_wizard/wizard.py

@@ -329,47 +329,42 @@ def __init__(self, wizard, parent):

        # toggles
        self.radioEpsg = wx.RadioButton(parent=self, id=wx.ID_ANY, label=_(
-            "Select coordinate reference system by EPSG"), style=wx.RB_GROUP)
+            "Select CRS from a list by EPSG or description"), style=wx.RB_GROUP)


"Select CRS from a list" should be the title of the EPSGPage as well.

petrasovaa · 2020-06-21T03:07:25Z

gui/wxpython/location_wizard/wizard.py

@@ -1486,41 +1481,23 @@ def OnEnterPage(self, event):
        event.Skip()

    def OnPageChanging(self, event):
-        if event.GetDirection() and not os.path.isfile(self.wktfile):


There needs to be a condition, you are trying to prevent from going to next page if there is no text (previously - if the file doesn't exist). Ideally it would check the WKT string is valid, but that's very difficult, so we will just check for any string. It's technically not needed because you already deal with that by disabling the next button, but let's keep it there.

I was surprised your current code works since you don't have any condition there (so it would prevent the page from switching every time), but that's because of the event.Skip() - you need to remove it from there. Please also remove event Skip from GeoreferencedFilePage (the same function), it's the same case, it shouldn't be there.

petrasovaa · 2020-06-21T03:09:49Z

gui/wxpython/location_wizard/wizard.py


                grass.create_location(dbase=self.startpage.grassdatabase,
                                      location=self.startpage.location,
-                                      wkt=self.wktpage.wktfile,
+                                      wkt=self.wktpage.wktstring,


The error you are getting comes from this function, it expects file not string. I created a separate PR #723, so you don't need to deal with that.

…e title of EPSGpage, checked for any wktstring in OnPageChanging function, removed event.skip() from WKTPage and GeoreferencedFilePage.

petrasovaa · 2020-06-21T18:36:03Z

gui/wxpython/location_wizard/wizard.py

@@ -1481,13 +1479,12 @@ def OnEnterPage(self, event):
        event.Skip()

    def OnPageChanging(self, event):
-        if event.GetDirection():
+        isblank = bool(self.wktstring and self.wktstring.strip())
+        if event.GetDirection() and isblank is False:


simply just if event.GetDirection() and not self.wktstring.strip(): should suffice.

I made changes in text and also changed WKT page to be similar as PRO…

ee7ad16

…J.4 page. Now I have a coding problem with passing wkt string as a stdin into the g.proj function.

lindakarlovska marked this pull request as draft June 19, 2020 10:36

petrasovaa linked an issue Jun 19, 2020 that may be closed by this pull request

[Feat] wxGUI loc wizard: Improve new location's CRS method selection page #706

Closed

6 tasks

petrasovaa added the gsoc Reserved for Google Summer of Code student(s) label Jun 19, 2020

petrasovaa requested changes Jun 19, 2020

View reviewed changes

wxGUI/Location wizard: Dropped 'Simple methods' and changed the title…

a36b93e

… of the page, used 'Additional methods' instead of 'Advanced additional methods', renamed 'customstring' to 'wktstring', removed a whitespace, changed run command of wkt selection in the summary page.

petrasovaa requested changes Jun 20, 2020

View reviewed changes

petrasovaa reviewed Jun 20, 2020

View reviewed changes

wxGUI/location wizard: Enabled scrolling of panelProj. Replaced 'wktf…

5517389

…ile' by 'wktstring'

petrasovaa mentioned this pull request Jun 21, 2020

pythonlib: allow wkt to be also string in create_location() #723

Merged

petrasovaa requested changes Jun 21, 2020

View reviewed changes

wxGUI/Location Wizard: renamed 'GetWktstring' to 'OnText', changed th…

1f11104

…e title of EPSGpage, checked for any wktstring in OnPageChanging function, removed event.skip() from WKTPage and GeoreferencedFilePage.

petrasovaa reviewed Jun 21, 2020

View reviewed changes

wxGUI/Location Wizard: Simplified testing of wktstring.

9cc46ff

petrasovaa approved these changes Jun 23, 2020

View reviewed changes

petrasovaa marked this pull request as ready for review June 23, 2020 02:31

petrasovaa merged commit f56d21d into OSGeo:master Jun 23, 2020

neteler added this to the 8.0.0 milestone Dec 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wxGUI loc wizard: Improve new location's CRS method selection page #721

wxGUI loc wizard: Improve new location's CRS method selection page #721

lindakarlovska commented Jun 19, 2020 •

edited

Loading

petrasovaa commented Jun 19, 2020

petrasovaa Jun 19, 2020

petrasovaa Jun 19, 2020

petrasovaa Jun 19, 2020

petrasovaa Jun 19, 2020

petrasovaa Jun 19, 2020

petrasovaa left a comment

petrasovaa Jun 20, 2020

petrasovaa Jun 21, 2020

petrasovaa Jun 21, 2020

petrasovaa Jun 21, 2020

petrasovaa Jun 21, 2020

petrasovaa Jun 21, 2020

wxGUI loc wizard: Improve new location's CRS method selection page #721

wxGUI loc wizard: Improve new location's CRS method selection page #721

Conversation

lindakarlovska commented Jun 19, 2020 • edited Loading

petrasovaa commented Jun 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrasovaa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lindakarlovska commented Jun 19, 2020 •

edited

Loading