-
Notifications
You must be signed in to change notification settings - Fork 1
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
CAS-61 Make the APIClient return model objects #81
Conversation
751307c
to
1a0bc2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I have a couple very minor questions, but no objections to merging
Represents the data object returned by the CAS API for nearest neighbor annotations. | ||
""" | ||
|
||
class DatasetStatistics(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly a dumb question: Are these classes nested because they are only used for fields of CellTypeSummaryStatisticsResults
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's exactly right
cellarium/cas/models.py
Outdated
Represents the data object returned by the CAS API for a ontology-aware annotations. | ||
""" | ||
|
||
class Matches(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this class represent multiple matches or just one match? If it's just one match, could it be Match
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gah, you're totally right
cellarium/cas/models.py
Outdated
(e.g. a query of the cell database using a matrix). | ||
""" | ||
|
||
class Matches(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question as the other Matches
class
03a54ff
to
2ab94eb
Compare
2ab94eb
to
6e66f2d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR makes it so that rather than returning
list
s ofdict
s, the API client will now return typed objects. This should make it much easier for users to develop against and understand the CAS api.Note: this is a breaking change! Results are returned in objects (as opposed to lists of objects) and the list is now in a field named
data
. This is true for all of the types of enumerations that are returned.For instance, if an endpoint returns:
and is returned as object
foo
,foo[0]
will fail butfoo.data
will return the list of objects. Furthermore, since these are typed objects, they can't be accessed with subtypes but must be accessed by the variable name.For instance, in the example above,
foo.data[0]["query_cell_id"]
will fail butfoo.data[0].query_cell_id
will return"1"
.The docs and example notebook were updated to reflect this