Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

gwrun argspec #30

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

gwrun argspec #30

wants to merge 12 commits into from

Conversation

kotfic
Copy link
Contributor

@kotfic kotfic commented Oct 1, 2018

This PR creates a decorator parameter that allows for arbitrary association of metadata with a callable's argument list.

This metadata is exposed through an attribute on the callable as a GWFuncDesc object which has an API for accessing argument metdata. E.G.

In [1]: from girder_worker_utils.decorators import parameter   

In [2]: @parameter('a', type=int, some_key='foo') 
   ...: @parameter('b', type=int, some_other_key='bar') 
   ...: def my_func(a, b): 
   ...:     return a + b 
   ...:                                                                                                                                                                                                                                 

In [4]: my_func._gw_function_description                                                                                                                                                                                                
Out[4]: <GWFuncDesc(a:POSITIONAL_OR_KEYWORD, b:POSITIONAL_OR_KEYWORD)>

In [5]: my_func._gw_function_description['a']                                                                                                                                                                                           
Out[5]: <girder_worker_utils.decorators.Arg at 0x7f102f763908>

In [6]: dir(my_func._gw_function_description['a'])                                                                                                                                                                                      
Out[6]: 
['__class__',
 ....
 'name',
 'some_key',
 'type']


In [7]: my_func._gw_function_description['a'].some_key                                                                                                                                                                                  
Out[7]: 'foo'

The ultimate goal is to use this metadata for automatic generation of CLI & Web UI tools from Girder Worker tasks

@kotfic
Copy link
Contributor Author

kotfic commented Oct 1, 2018

For a sense of how this is going to be used, see: girder/girder_worker#314

if isinstance(doc, bytes):
doc = doc.decode('utf-8')
else:
doc = cleandoc(doc)
Copy link

Choose a reason for hiding this comment

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

Doesn't getdoc do this already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, not sure where that came from

return self._construct_argument(
self._get_class(self._signature.parameters[key]), key)

def _construct_argument(self, cls, name, **kwargs):
Copy link

Choose a reason for hiding this comment

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

**kwargs seems to be pointless here. Also, I would avoid the cls parameter name because it makes the function read as a class method below.

girder_worker_utils/decorators.py Show resolved Hide resolved
girder_worker_utils/decorators.py Show resolved Hide resolved
raise TypeError('Expected argument name to be a string')

data_type = kwargs.get("data_type", None)
if data_type is not None and callable(data_type):
Copy link

Choose a reason for hiding this comment

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

Why not just if callable(data_type):?

girder_worker_utils/decorators.py Show resolved Hide resolved
def description():
return getattr(func, GWFuncDesc._func_desc_attr)

func.description = description
Copy link

Choose a reason for hiding this comment

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

Why have both this and GWFuncDesc.get_description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mainly for compatibility with code that's already been written (e.g. item_tasks?) I can just drop it if nothing is actually using it

Handling instance bound variables (i.e. 'self') will have to be solved
in the CLI.
@kotfic kotfic changed the title [WIP] gwrun argspec gwrun argspec Oct 24, 2018
@kotfic
Copy link
Contributor Author

kotfic commented Oct 24, 2018

I'm happy to roll 99cd62d back if we don't want to add the deprecation mechanism. It just seems like a better way to signal to downstreams that we're moving away from something. It might not make the most sense to start with girder_worker_utils, but GWU has been providing a nice test bed for stuff like this

This allows metadata mapping for each type of argument to be defined on
the consumers' (e.g. CLI) classes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants