ask_async #102

sunildkumar · 2023-10-09T22:59:16Z

This enables customers to query groundlight asynchronously. It adds want_async as a new parameter on submit_image_query and implements ask_async, a convenient interface to submit queries asynchronously.

Blockers to merging this:
~~1. Merge my corresponding server-side PR the enables async IQ submissions.~~
~~2. Make changes based on updates to the BE~~
3. Fix PR comments
4. Show that new SDK tests pass after merge, right now they fail because the server side code to support this feature isn't deployed.
5. How do we want to document this added functionality? I will have to add to the docs reference in sphinx once @blaise-muhirwa sets that up :). I should definitely show a small example here about how you need to query for results after querying async.
6. Do we want to add a section to docs/building-applications with a small demo of how one can use ask_async?

src/groundlight/client.py

robotrapta

This LGTM - some suggestions within, and of course we should be conscious of dependencies when merging.

Also, this probably deserves a major version bump to 0.12, especially with the other ask_ methods.

spec/public-api.yaml

src/groundlight/client.py

robotrapta · 2023-10-11T18:20:19Z

src/groundlight/client.py

+                raise ValueError(
+                    "wait must be set to 0 to use want_async. Using wait and want_async at the same time is incompatible."  # noqa: E501
+                )
+            params["want_async"] = want_async


I think here we should be more deliberate about what we send to the API. We can be pretty flexible in what the SDK method uses to indicate that they want async - they can give us an actual bool, or a 0/1 int or something else which indicates truthiness. But we should be more specific about what we send to the server. In the other PR I argue for always sending “1” or “0”.

Why not just True/False?

I'd defer to Leo. My understanding is the the "1" "0" distinction fixes the bits representation. True values can have multiple representations (that occasionally are different by system) so we might as well choose a 1:1 mapping on the values sent into our system rather than a many to one mapping.
Again, I defer to Leo

I disagree with this pretty strongly as we are going to essentially going to have to write a two translation layers (on both the SDK and BE) to convert to and from these "string bools". OTOH, python's True and False are already unambiguous values for T/F. Additionally the SDK automatically handles serialization of True/False and our BE automatically handles deserialization to True/False so no custom translation is required.

If we really want this, I think we should do it as its own PR as that way we can focus on applying this update uniformly as there is BE work required as well.

If we want to accept other arguments to represent truthiness from the user I think the solution is to do validation on the inputs prior to whatever business logic we do.

Suggested change

params["want_async"] = want_async

params["want_async"] = str(bool(want_async))

is what I mean. Don't rely on the variable's python-truthiness and its string representation being the same.

src/groundlight/client.py

blaise-muhirwa · 2023-10-12T01:58:40Z

.vscode/settings.json

-  "python.analysis.extraPaths": ["./generated"],
-  "python.formatting.provider": "black"
-}
+  "editor.rulers": [


Looks like you accidentally committed this!

this was intentional - I was just adding the stuff we have in zuuul that we needed here to

make it easier to keep code linted

make black behave rationally

...at least on my machine. the rulers are helpful to make the linter happy and black does wild things with the imports if it is allowed to touch them.

blaise-muhirwa · 2023-10-13T19:31:43Z

src/groundlight/client.py

        if human_review is not None:
            params["human_review"] = human_review

+        if want_async is True:


Suggested change

if want_async is True:

if want_async:

blaise-muhirwa

Awesome! As far as documenting this with sphinx, it won't be a problem since sphinx autogenerates docs for everything under client.py. Just to make sure things look good tho you can run make develop-docs-comprehensive to make sure the docs look good!

…folk's docstrings

…o expect a new param

…d code to expect a new param" This reverts commit 2477fd5.

This reverts commit cb9359e.

This reverts commit 935c036.

…generated code to expect a new param"" This reverts commit 07670e3.

…d code to expect a new param" This reverts commit 2477fd5.

…n other folk's docstrings" This reverts commit 67e3edd.

* bump version * Update UNSURE to be UNCLEAR, catch __USURE received from the service (#105) * ask_async (#102) * initial commit * added ask_async to submit_image_query * Automatically reformatting code * added ask async method * Automatically reformatting code * added integration tests (requires BE merge first) * Automatically reformatting code * satisfying mypy * Automatically reformatting code * fix comments * change what type of exception test is catching * Automatically reformatting code * fix imports organization issue * fix implementation - wait must be 0 not None * Automatically reformatting code * forgot to make wait=0 in relevant test * feedback from PR review * Automatically reformatting code * ensure want_async is a serializable bool * add description * updated sphinx reqs to render some of the dependencies * updated docstring for ask_async and fixed small sphinx bugs in other folk's docstrings * Tests aren't passing because I didn't update the autogenerated code to expect a new param * Revert "Tests aren't passing because I didn't update the autogenerated code to expect a new param" This reverts commit 2477fd5. * fix generated * Automatically reformatting code * fix lint * Automatically reformatting code * Revert "Automatically reformatting code" This reverts commit cb9359e. * Revert "fix generated" This reverts commit 935c036. * Revert "Revert "Tests aren't passing because I didn't update the autogenerated code to expect a new param"" This reverts commit 07670e3. * Revert "Tests aren't passing because I didn't update the autogenerated code to expect a new param" This reverts commit 2477fd5. * Revert "updated docstring for ask_async and fixed small sphinx bugs in other folk's docstrings" This reverts commit 67e3edd. * third time at generated docs is the charm --------- Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> * Cli polishing (#109) * Add basic catch if api token isn't specified when cli is called * Pushes Groundlight class instantiation up until the function is actually called with arguments. This means that the entire help args are available even if we can't instantiate the class (no api key) * Fixed misunderstanding with metaprogramming, added tests * Addressing comments --------- Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> * Add ask_confident and ask_ml (#99) * Adding ask_confident and ask_fast * Fixing ask_ml behavior * Unhide wait functions, merging logic, fixed iq_is_answered logic * Rewriting doc strings in Sphinx style * fixed sphinx docstring return types * Making iq submission with inspection work with newly optional patience time --------- Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> * Linting * fix ask_async docstring --------- Co-authored-by: brandon <brandon@groundlight.ai> Co-authored-by: Brandon <132288221+brandon-groundlight@users.noreply.github.com> Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai>

* bump version * Update UNSURE to be UNCLEAR, catch __USURE received from the service (#105) * rough draft of demo * ask_async (#102) * initial commit * added ask_async to submit_image_query * Automatically reformatting code * added ask async method * Automatically reformatting code * added integration tests (requires BE merge first) * Automatically reformatting code * satisfying mypy * Automatically reformatting code * fix comments * change what type of exception test is catching * Automatically reformatting code * fix imports organization issue * fix implementation - wait must be 0 not None * Automatically reformatting code * forgot to make wait=0 in relevant test * feedback from PR review * Automatically reformatting code * ensure want_async is a serializable bool * add description * updated sphinx reqs to render some of the dependencies * updated docstring for ask_async and fixed small sphinx bugs in other folk's docstrings * Tests aren't passing because I didn't update the autogenerated code to expect a new param * Revert "Tests aren't passing because I didn't update the autogenerated code to expect a new param" This reverts commit 2477fd5. * fix generated * Automatically reformatting code * fix lint * Automatically reformatting code * Revert "Automatically reformatting code" This reverts commit cb9359e. * Revert "fix generated" This reverts commit 935c036. * Revert "Revert "Tests aren't passing because I didn't update the autogenerated code to expect a new param"" This reverts commit 07670e3. * Revert "Tests aren't passing because I didn't update the autogenerated code to expect a new param" This reverts commit 2477fd5. * Revert "updated docstring for ask_async and fixed small sphinx bugs in other folk's docstrings" This reverts commit 67e3edd. * third time at generated docs is the charm * Automatically reformatting code * finish making tests work * Automatically reformatting code --------- Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> * Cli polishing (#109) * Add basic catch if api token isn't specified when cli is called * Pushes Groundlight class instantiation up until the function is actually called with arguments. This means that the entire help args are available even if we can't instantiate the class (no api key) * Fixed misunderstanding with metaprogramming, added tests * Addressing comments --------- Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> * Add ask_confident and ask_ml (#99) * Adding ask_confident and ask_fast * Automatically reformatting code * Fixing ask_ml behavior * Adding to test * Automatically reformatting code * set default wait for ask_ml * Unhide wait functions, merging logic, fixed iq_is_answered logic * Automatically reformatting code * Rewriting doc strings in Sphinx style * ask_fast to ask_ml in the tests * fixed sphinx docstring return types * Cleaning the lint trap * Last bits of lint * Making iq submission with inspection work with newly optional patience time * single char typo * Reorder functions to trick Git's LCS alg to be correct * Automatically reformatting code --------- Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> * Linting * addressed self comments * fix ask_async docstring * small fixes * cleaning up a bit * Update docs/docs/building-applications/5-async-queries.md Co-authored-by: blaise-muhirwa <135643310+blaise-muhirwa@users.noreply.github.com> * Update docs/docs/building-applications/5-async-queries.md Co-authored-by: robotrapta <79607467+robotrapta@users.noreply.github.com> * small fixes based on PR feedback from Leo and Blaise --------- Co-authored-by: brandon <brandon@groundlight.ai> Co-authored-by: Brandon <132288221+brandon-groundlight@users.noreply.github.com> Co-authored-by: Auto-format Bot <autoformatbot@groundlight.ai> Co-authored-by: blaise-muhirwa <135643310+blaise-muhirwa@users.noreply.github.com> Co-authored-by: robotrapta <79607467+robotrapta@users.noreply.github.com>

sunildkumar and others added 7 commits October 9, 2023 15:58

initial commit

ec65a47

added ask_async to submit_image_query

98307c1

Automatically reformatting code

2b3d07e

added ask async method

b174917

Automatically reformatting code

56086bb

added integration tests (requires BE merge first)

57b8fa2

Automatically reformatting code

b76975d

sunildkumar commented Oct 10, 2023

View reviewed changes

src/groundlight/client.py Show resolved Hide resolved

sunildkumar and others added 9 commits October 9, 2023 17:14

satisfying mypy

038631a

Automatically reformatting code

8bd7c02

fix comments

a07aaa0

change what type of exception test is catching

aaac63f

Automatically reformatting code

e14254b

fix imports organization issue

6872eff

fix implementation - wait must be 0 not None

d53c9bc

Automatically reformatting code

e7563d2

forgot to make wait=0 in relevant test

4e7207b

sunildkumar requested review from blaise-muhirwa, brandon-wada, mjvogelsong and robotrapta and removed request for mjvogelsong and robotrapta October 10, 2023 00:57

robotrapta approved these changes Oct 11, 2023

View reviewed changes

blaise-muhirwa reviewed Oct 12, 2023

View reviewed changes

sunildkumar and others added 3 commits October 12, 2023 16:02

feedback from PR review

d1521f4

Automatically reformatting code

c78b72b

Merge branch 'main' into ask_async

13c1aab

brandon-wada approved these changes Oct 13, 2023

View reviewed changes

brandon-wada changed the base branch from main to version_0.12.0 October 13, 2023 17:19

blaise-muhirwa reviewed Oct 13, 2023

View reviewed changes

blaise-muhirwa approved these changes Oct 13, 2023

View reviewed changes

sunildkumar and others added 19 commits October 13, 2023 12:39

ensure want_async is a serializable bool

5521853

add description

1dd844e

updated sphinx reqs to render some of the dependencies

5f8dc21

updated docstring for ask_async and fixed small sphinx bugs in other …

67e3edd

…folk's docstrings

Tests aren't passing because I didn't update the autogenerated code t…

2477fd5

…o expect a new param

Revert "Tests aren't passing because I didn't update the autogenerate…

07670e3

…d code to expect a new param" This reverts commit 2477fd5.

fix generated

935c036

Automatically reformatting code

cb9359e

fix lint

f56b647

Automatically reformatting code

11b5e3a

Revert "Automatically reformatting code"

2374657

This reverts commit cb9359e.

Revert "fix generated"

7098f4c

This reverts commit 935c036.

Revert "Revert "Tests aren't passing because I didn't update the auto…

f589628

…generated code to expect a new param"" This reverts commit 07670e3.

Revert "Tests aren't passing because I didn't update the autogenerate…

f0f9f72

…d code to expect a new param" This reverts commit 2477fd5.

Revert "updated docstring for ask_async and fixed small sphinx bugs i…

5b10deb

…n other folk's docstrings" This reverts commit 67e3edd.

third time at generated docs is the charm

532bb3e

Automatically reformatting code

2318671

finish making tests work

52f4503

Automatically reformatting code

fa45a7f

sunildkumar merged commit 0a7b928 into version_0.12.0 Oct 16, 2023

sunildkumar deleted the ask_async branch October 16, 2023 23:48

	params["want_async"] = want_async
	params["want_async"] = str(bool(want_async))

ask_async #102

ask_async #102

Uh oh!

Conversation

sunildkumar commented Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

robotrapta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blaise-muhirwa left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sunildkumar commented Oct 9, 2023 •

edited

Loading