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

Provide clean process and test code as examples #3

Open
huard opened this issue Feb 20, 2018 · 8 comments
Open

Provide clean process and test code as examples #3

huard opened this issue Feb 20, 2018 · 8 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Milestone

Comments

@huard
Copy link
Collaborator

huard commented Feb 20, 2018

What I want to do here is propose a few good practices that experience has shown work well. Now I personally don't have that much experience, so I'm not in such a good position to do this...

Here are things that I think matter when we write processes:

  • make processes easy to use to define other similar processes (inheritance)
  • metadata is being used to construct the documentation, as well as the user interface. It needs to be complete, and be uniform so its useful to UI developers.
  • make sure that all processes log roughly the same information to facilitate debugging.
  • use adequate mime-types to facilitate UI interfacing

Then what do we test exactly? I think WPS processes should remain simple. The complexity should be in stand-alone functions that can be tested independently from the WPS context.

Comments, suggestions ?

@huard huard self-assigned this Feb 20, 2018
@huard huard added the enhancement New feature or request label Feb 20, 2018
@huard huard added this to the Montreal 2018 milestone Feb 20, 2018
@cehbrecht cehbrecht added this to To Do in Dar-es-Salaam: September 2018 Release via automation Mar 28, 2018
@huard
Copy link
Collaborator Author

huard commented Apr 11, 2018

So for example, to make processes modular, how should we define inputs in the init ?

  • Define them as named class objects (pollutes class namespace, unless we use underscores)
  • Define a method that returns the inputs, and can be called by
  • Define them outside the Process class and just refer to them

@cehbrecht
Copy link
Member

@huard not sure if it fits to what you want, but to separate the process code from the process definition we can use the OWSContext: geopython/pywps#322

But we don't have support for it yet.

@cehbrecht
Copy link
Member

... following up. We don't have good examples but the aim is to separate the processing functionality from the process input/output handling. In Hummingbird we have a processing module which is imported by the process definition:

https://github.com/bird-house/hummingbird/blob/f2ff85cdfaf53d86e91b3b62cc4fd8897832f63e/hummingbird/processes/wps_cmor_checker.py#L4

Or the process is just a wrapper around an existing library like for the ioos compliance checker:
https://github.com/bird-house/hummingbird/blob/f2ff85cdfaf53d86e91b3b62cc4fd8897832f63e/hummingbird/processes/wps_compliance_checker.py#L4

I also had the idea of using Python decorators to enable a normal python function as wps process:

  @wps
  def say_hello(name='Ada'):
          """
          :param name: String. Your name. Default: Ada
          :return: String. A friendly message.
          """
         return "Hello {}".format(name)

The decorator can use the information given by the function definition (name, input, output, abstract, type, ....) to generate a WPS process ... or an OWSContext document.

See also: http://wiki.rsg.pml.ac.uk/pywps/PyWPS_4.0_Ideas.html#Decorators

But that is for the future :)

@tomLandry
Copy link

tomLandry commented Apr 12, 2018 via email

@huard
Copy link
Collaborator Author

huard commented Apr 13, 2018

Fully agree with keeping the WPS class as thin as possible, and put the logic behind a processing module. My question is rather with the WPS interface itself. For example, the new EO processes all share a basic set of inputs and outputs. When I started fixing grammar and syntax, I realized I would have to make the same corrections to four different processes. I think there is a better way.

One option would be to have inputs.py and outputs.py modules inside the process/ directory, where we define all the LiteralInput and ComplexInput that are used more than once, and just reference them in the class init.

We could also bind functions to these classes to convert the input from the string input into whatever is needed. For example, imagine we have an inputs.py file with:

bbox = \
    LiteralInput('BBox', 'Bounding Box',
                 data_type='string',
                 abstract="Enter a bbox: min_lon, max_lon, min_lat, max_lat."
                          " min_lon=Western longitude,"
                          " max_lon=Eastern longitude,"
                          " min_lat=Southern or northern latitude,"
                          " max_lat=Northern or southern latitude."
                          " For example: -80,50,20,70",
                 min_occurs=1,
                 max_occurs=1,
                 default='-180,180,-90,90',
                 )

def convert_bbox(self, request):
    """Convert bbox string into reordered float."""
    s = request.inputs[self.identifier][0].data
    b = map(float, s.split(','))
    return b[0], b[2], b[1], b[3]

bbox.convert = types.MethodType(convert_bbox, bbox)

We can use this bbox instance in mutiple processes. Do you think we have to deep copy instances first ? Could there be clashes if the same instance is used in multiple processes ? Another option is to store the keyword dictionary in inputs.py and instance the class in each process.

@cehbrecht
Copy link
Member

I think I understand what you want. Python has probably an answer for us. I try to figure it out.

@huard
Copy link
Collaborator Author

huard commented Nov 1, 2018

One thing I've tried is to create the inputs and outputs as class attributes.

class TestProcess:
    identifier = 'test'
    name = LiteralInput(...)

    def __init__(self):
        inputs = [self.name,]
        ...  

which let's you replace some inputs and outputs in a subclass but does not require to rewrite identical ones. Seems simple enough.

@cehbrecht cehbrecht added this to To do in Hot Topics via automation Dec 18, 2018
@cehbrecht cehbrecht modified the milestones: 0.4.0, 1.0.0 Apr 17, 2019
@huard
Copy link
Collaborator Author

huard commented Oct 7, 2019

I've taken the habit of creating a wpsio.py module, defining all the inputs and outputs that are reused in different modules. Simple, easy, works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Hot Topics
  
To do
Development

No branches or pull requests

3 participants