Conversation
odm2api/ODM2/services/readService.py
Outdated
|
|
||
| # 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.
| 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. |
emiliom
left a comment
emiliom
left a comment
There was a problem hiding this comment.
Looks good. I'll merge in a bit.
|
I'll merge when the CI is completed. |
|
Thanks @emiliom! 👍 |
2 participants
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.