Fix builtins #137
Fix builtins #137
Conversation
odm2api/ODM2/services/readService.py
Outdated
| @@ -379,21 +385,25 @@ def getVariables(self, ids=None, codes=None, sitecode=None, results=False): | |||
| return None | |||
|
|
|||
| # Method | |||
| def getMethods(self, ids=None, codes=None, type=None): | |||
| def getMethods(self, ids=None, codes=None, medtype=None, **kwargs): | |||
There was a problem hiding this comment.
medtype seems like a misspelling, or else med doesn't seem like a good shorthand for "method". How about mettype or even better, full blown methodtype?
There was a problem hiding this comment.
I'll change to methodtype. This is the most clear.
odm2api/ODM2/services/readService.py
Outdated
| @@ -155,34 +157,38 @@ def getAnnotations(self, type=None, codes=None, ids=None): | |||
| """ | |||
| # TODO What keywords do I use for type. | |||
| a = Annotations | |||
| if type: | |||
| if type == 'action': | |||
| if 'type' in kwargs: | |||
There was a problem hiding this comment.
Should there be a warning if any other kwarg is passed, besides type? ie, an unhandled/unrecognized kwarg name.
Not just in this function, but everywhere you use this scheme.
There was a problem hiding this comment.
I'll see how to best handle unrecognized kwarg
Very nice scheme!! I really like it, specially how it creates a smooth deprecation path. I made a suggestion or two. I'll hold off on merging until you've had a chance to follow up. |
|
I'll merge when the CI is completed. |
|
Thanks @emiliom! 👍 |
Overview
This PR addresses #115. I simply put warnings to avoid breaking the API. All older code that uses the builtin arguments should still work.