add method for exporting predictions#300
Conversation
sasha-scale
requested review from
drakejwong,
gatli,
jean-lucas,
pfmark and
tetranoir
jean-lucas
left a comment
jean-lucas
left a comment
There was a problem hiding this comment.
looks good, could you also update the version + changelog?
| Returns: | ||
| Response payload as JSON dict. | ||
| """ | ||
| print(payload, route) |
There was a problem hiding this comment.
🙏 finally haha i was looking for this
nucleus/slice.py
Outdated
| route=f"slice/{self.id}/{model.id}/exportForTraining", | ||
| requests_command=requests.get, | ||
| ) | ||
| print(api_payload) |
There was a problem hiding this comment.
| print(api_payload) |
nucleus/utils.py
Outdated
|
|
||
|
|
||
| def convert_export_payload(api_payload): | ||
| def convert_export_payload(api_payload, model_id: str = None): |
There was a problem hiding this comment.
Would make more sense to pass a boolean instead of nullable string here. Since we're not using the
value of model_id anywhere.
ie: convert_export_payload(api_payload, has_predictions: bool = False):
nucleus/slice.py
Outdated
| ) | ||
| return convert_export_payload(api_payload[EXPORTED_ROWS]) | ||
|
|
||
| def items_and_predictions( |
There was a problem hiding this comment.
In general I think it would be easier to use if we had the same interface for exporting items, predictions and annotations between a slice and a dataset. Right now the corresponding method in a dataset is export_predictions(model). I'd make this method consistent with the dataset one.
Just wondering in case we have a huge export, wouldn't we also run into timeout issues then with this request?
jean-lucas
left a comment
jean-lucas
left a comment
There was a problem hiding this comment.
thanks for addressing the comments. LGTM once conflicts are resolved 👍
3 participants
No description provided.