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

g.gui.gmodeler: add PyWPS export option #1919

Merged
merged 27 commits into from
Oct 11, 2022

Conversation

pesekon2
Copy link
Contributor

@pesekon2 pesekon2 commented Oct 2, 2021

No description provided.

@pesekon2 pesekon2 added enhancement New feature or request Python Related code is in Python labels Oct 2, 2021
@pesekon2 pesekon2 requested a review from landam October 2, 2021 17:30
@pesekon2 pesekon2 self-assigned this Oct 2, 2021
@pesekon2
Copy link
Contributor Author

pesekon2 commented Oct 2, 2021

Figures containing screenshots of the Python editor should be updated - is there anyone who has the same visual as those that are already there? The GUI looks a bit different in my case and it would be nice to have the look more-or-less standardized (and I would like to avoid taking all the pictures again). @landam?

EDIT: Okay, I'll put them there myself.

@pesekon2 pesekon2 changed the title add PyWPS export option to gmodeler g.gui.gmodeler: add PyWPS export option Oct 3, 2021
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

minor manual fixes - the rest, I didn't check

gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
Copy link
Member

@landam landam left a comment

Choose a reason for hiding this comment

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

  • reorder buttons (combox on the left, refresh+save+run on the right
  • update documentation (update + add new screenshots)

@landam landam added the GUI wxGUI related label Oct 21, 2021
@landam landam added this to the 8.2.0 milestone Nov 28, 2021
@landam
Copy link
Member

landam commented Nov 28, 2021

Looks nice

modeler-wps

    process = Model()

    processes = [Model()]
    application = Service(processes)

@pesekon2 Question: What is purpose of line

 process = Model()

?

@landam
Copy link
Member

landam commented Nov 28, 2021

Suggestion: line

from pywps.app.Service import Service

could be moved into if __name__ block.

@landam
Copy link
Member

landam commented Nov 28, 2021

Also consider improving comment

# here you could also specify the GRASS location, for example:
  1. You must specify GRASS location
  2. Either by providing path to the existing GRASS location
  3. Or by EPSG code in order to create a new one

Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

tiny grammar fixes :)

gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
gui/wxpython/gmodeler/g.gui.gmodeler.html Outdated Show resolved Hide resolved
@pesekon2
Copy link
Contributor Author

pesekon2 commented Dec 2, 2021

    process = Model()

    processes = [Model()]
    application = Service(processes)

@pesekon2 Question: What is purpose of line

 process = Model()

?

@landam: Probably just some residuum from one of the previous versions. Fixed in 024d3cd, thanks for pointing it out.

@pesekon2
Copy link
Contributor Author

pesekon2 commented Dec 2, 2021

Suggestion: line

from pywps.app.Service import Service

could be moved into if __name__ block.

@landam: Moved in bdc5f35

@pesekon2
Copy link
Contributor Author

@landam The code has been changed (see 7028d12) in the following way: Output options are skipped if they are intermediate or (blank and unparameterized).

@pesekon2 pesekon2 requested a review from landam May 23, 2022 11:43
@landam
Copy link
Member

landam commented May 25, 2022

Great, now only relevant output parameter is included:

        inputs.append(LiteralInput(
            identifier="vbuffer1_distance",
            title="Buffer distance along major axis in map units",
            data_type="float",
            default=1000))




        outputs.append(ComplexOutput(
            identifier="rslopeaspect4_slope",
            title="Name for output slope raster map",
            supported_formats=sup_formats,
            default="slope"))

I just wonder why there are so many empty lines. @pesekon2 I would suggest to keep it consistent (1/2 lines). Similarly here:

        return response



if __name__ == "__main__":

@landam
Copy link
Member

landam commented May 25, 2022

@pesekon2 Unfortunately the process fails with:

Traceback (most recent call last):
  File "/home/martin/Downloads/pywps-flask-master/demo.py", line 53, in <module>
    Model()
  File "/home/martin/Downloads/pywps-flask-master/processes/pywps_grass_test.py", line 29, in __init__
    outputs.append(ComplexOutput(
TypeError: __init__() got an unexpected keyword argument 'default'

Tested with pywps versions 4.2.11 and 4.5.2.

@pesekon2
Copy link
Contributor Author

pesekon2 commented Jun 5, 2022

@landam According to the PyWPS documenatation, PyWPS 4.x does not support parameter default for ComplexOutput objects. Updated in 2fc8692 to skip this parameter for ComplexOutput objects - fortunately the code is written in a way to handle it later in _handler, so the default value is still used, just later.

@pesekon2 pesekon2 requested a review from veroandreo June 6, 2022 12:25
@landam
Copy link
Member

landam commented Aug 29, 2022

Currently it's failing with

 File "/home/martin/src/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/gmodeler/model.py", line 2837, in _write_input_outputs
    self._write_input_output_object(
TypeError: WritePyWPSFile._write_input_output_object() missing 1 required positional argument: 'value'

@pesekon2 Method WritePyWPSFile._write_input_output_object() has 7 arguments (https://github.com/pesekon2/grass/blob/gmodeler_export_pywps/gui/wxpython/gmodeler/model.py#L2843) defined, but only called with 6 arguments (https://github.com/pesekon2/grass/blob/gmodeler_export_pywps/gui/wxpython/gmodeler/model.py#L2837).

@pesekon2
Copy link
Contributor Author

pesekon2 commented Sep 8, 2022

@landam: Thanks for noticing that. The origin of this bug is actually sort of funny. The latest state of this PR wasn't cool enough according to the Black code style suppression, so I pushed a minimal reformatting commit 8868761. Apparently, it made the code stylish enough, but a small typo broke it to be unusable.

(what to learn: test the code even after those stupid reformatting commits)

Fixed in 0282c38.

@landam
Copy link
Member

landam commented Sep 14, 2022

New tasks based on our testing

  • add overwrite flag to r.out.gdal command (only if overwrite is enabled in preferences)
  • limit supported formats (raster -> image/tif, vector -> application/gml+xml)

@pesekon2
Copy link
Contributor Author

landam:
add overwrite flag to r.out.gdal command (only if overwrite is enabled in preferences)

Solved in a249d85

@pesekon2
Copy link
Contributor Author

landam:
limit supported formats (raster -> image/tif, vector -> application/gml+xml)

Solved in b176b82

@landam
Copy link
Member

landam commented Oct 8, 2022

I tested sample model with pywps 4.5.2 and it works.

@landam landam self-requested a review October 8, 2022 16:01
@landam
Copy link
Member

landam commented Oct 8, 2022

@veroandreo Can we merge this PR?

@pesekon2 pesekon2 merged commit 0945054 into OSGeo:main Oct 11, 2022
@pesekon2 pesekon2 deleted the gmodeler_export_pywps branch October 11, 2022 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request GUI wxGUI related Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants