Conversation
sreeder
requested review from
Elijahwalkerwest,
emiliom,
horsburgh,
lsetiawan and
ocefpaf
odm2api/ODM2/services/readService.py
Outdated
| """Retrieve a list of Sampling Feature objects. | ||
| Retrieve a list of Datasets associated with the given sampling feature data. | ||
|
|
||
| Must specify either samplingFeatureID OR samplingFeatureUUID OR samplingFeatureCode) |
There was a problem hiding this comment.
Thanks for following the convention that we established when you were gone. 😄 Maybe this line should be bold, so it's not easy to miss?
There was a problem hiding this comment.
@lsetiawan do I do that by adding the three > in front of it? and the whole paragraph or just the line that indicates you much specify one of the three features?
There was a problem hiding this comment.
I think just the line that indicates that you must specify one of three features. I think you can just do
**Must specify either samplingFeatureID OR samplingFeatureUUID OR samplingFeatureCode)**
There was a problem hiding this comment.
@lsetiawan okay, that has been updated. The formatting in the function documentation is just markdown right?
odm2api/ODM2/services/readService.py
Outdated
| print('Error running Query {}'.format(e)) | ||
| return None | ||
|
|
||
| def getSamplingFeatureDatasets(self, ids=None, codes=None, uuids=None, type=None): |
There was a problem hiding this comment.
I see that type is used here as a variable, it is a python reserved word. Maybe as we continue to develop the api, we should try to follow better coding convention. So since this is a new function, type should be replaced by dstype or something along those lines to avoid using reserved python vocab.
👍 |
lsetiawan
left a comment
lsetiawan
left a comment
There was a problem hiding this comment.
Thanks for making the suggested changes! The code now looks good to me. 👍
|
@sreeder How come this goes into |
|
@lsetiawan The way we have merged code in the past is: we do our fixes on their own small branches then merge them to development. Then we only merge into master when we are ready to create a new release. So development is the most recent up to date code and the master is stable. Was this workflow changed while I was gone? @horsburgh @emiliom |
|
Ah, interesing. I have not been working that way at all. I merge directly to |
|
This goes back to the back-and-forth about dev workflows we had a couple of weeks ago in the context of yoda-tools. @lsetiawan, as we discussed last week, for the remainder of this month (and the BiGCZ funding), let's follow the repo maintainer's preferred dev workflow. Which means, eg for odm2api, what @sreeder described; and, eg, or WOFpy and ODM2REST API, the one @lsetiawan described. |
|
@emiliom Sounds good. Thanks for the clarification. |
4 participants
Resolve issue #120. Created a new function getSamplingFeatureDatasets. It returns a DataSetsResults object that has access to both the Datasets object and the Results object