keepsake
keepsake copied to clipboard
replicate
Add status to experiments in Python API
The CLI command replicate ls has a STATUS column, but that's missing in the Python API. We should add that.
Hi @andreasjansson, Can I take up this? I have already started reading how replicate ls has been implemented in go and the Python code which will be needed as well.
@Ronit-j Feel free to tackle anything! :)
Andreas is working on a refactoring of the Go/Python interface at the moment, but I think this could be implemented entirely in Python-land. Either way I'm sure it won't be too hard to merge.
There is is_running(), which is kind of this. I think this issue is mainly that some kind of indication of status doesn't exist on replicate.experiments.list().
I wonder whether it makes sense to keep it as a function in Python. Semantically, it is doing a calculation, and isn't a field on the underlying data model. I could imagine us adding experiment.get_status(), or something.
@bfirsh If status is added as an attribute to Experiment and then updated when getting experiment details from the daemon it doesn't seem necessary. It seems there was a PR merged for this before #393, which is where is_running() was added. I think only part of this issue missing for now is the adding attribute status and it being present in repr.
Either a property or a function could work. Typically things which hit the disk/network are functions, to indicate to the user that this thing might take some time or throw an exception. I think we have to hit disk/network to get the heartbeat. Also, as I said above, it's doing a calculation on the underlying data, which semantically makes sense to be a function. Those things are making me lean towards it being a function.
Saying that, we have is_running() already though, which is already a status function under a different name, which is a bit weird. I can envisage "status" being more than two values in the future though, so maybe we deprecate is_running() and add a status() (or get_status()?) function to replace it, to be consistent with the CLI.
Ah looking at https://github.com/replicate/replicate/pull/393 I see now that @andreasjansson suggested is_running(), haha. Sorry for jerking you around here.
I think that should be called status() to be consistent with the CLI and to open up possibility for future other statuses.
So, I think the solution here is to make a new function that returns constants, and leave is_running() in place (but maybe indicate it is being deprecated, or just remove from docs, or something).