Skip to content
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

DOCS-2899: Edit code samples from Data client QA #3457

Merged
merged 7 commits into from
Sep 19, 2024

Conversation

sguequierre
Copy link
Collaborator

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Sep 13, 2024
if not tabular_data:
break
my_data.extend(tabular_data)
left_motor_filter = create_filter(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styling issue

component_name="motor-1"
)

data, count, last = await data_client.tabular_data_by_filter(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think showing the loop is probably good because it's a more realistic use case. most people will use this to get more than 50 data points.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 35 to 36
for tab in data:
print(tab)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the variable naming here makes sense. what does tab stand for tab? Maybe reading?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tabular data

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

tabular_data = await data_client.tabular_data_by_mql(
organization_id="<YOUR-ORG-ID>",
mql_binary=binary_pipeline
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styling

@@ -88,17 +89,40 @@ Obtain unified tabular data and metadata, queried with MQL.
```python {class="line-numbers linkable-line-numbers"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused with this, this feels like it's become a much more involved code sample. And we now no longer show how it works with pymongo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was required to make it work and I am still using pymongo-- it wasn't working for me otherwise

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you are indeed using pymongo. Which means the code now doesn't work for the bson package.
I just tested both code snippets again and they do work.

Did you run pip uninstall pymongo and pip install bson when you were using the bson package code? Can you try these again and let me know if they work? If not lets look into it together. The code snippets as they are are a lot shorter and cleaner so if possible I'd like to stick with them

Maybe we can make that clearer like this?

# using bson package (pip install bson)
import bson
tabular_data = await data_client.tabular_data_by_mql(org_id="<your-org-id>", mql_binary=[
    bson.dumps({ '$match': { 'location_id': '<location-id>' } }),
    bson.dumps({ '$limit': 5 })
])

# using pymongo package (pip install pymongo)
import bson
tabular_data = await data_client.tabular_data_by_mql(org_id="<your-org-id>", mql_binary=[
    bson.encode({ '$match': { 'location_id': '<location-id>' } }),
    bson.encode({ '$limit': 5 })
])

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the first option to work with organization_id. The second option as far as i can tell with pymongo still needs to be bson.BSON to work properly. Editing as such

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah true I missed the organization_id bit when copying - I'll send you a code snippet.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bson.BSON.encode works, but weirdly so does running it without. I'd love to see what error you're getting but this is fine to merge

my_data.extend(data)
camera_filter = create_filter(
component_name="camera-1"
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styling

filter=my_filter,
limit=20,
include_binary_data=False
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styling

org_id="a12b3c4e-1234-1abc-ab1c-ab1c2d345abc", days_of_data_to_delete)
organization_id="<YOUR-ORG-ID>",
delete_older_than_days=150
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styling


my_filter = create_filter(component_name="camera-1", organization_ids=["<YOUR-ORG-ID>"])
binary_metadata = await data_client.binary_data_by_filter(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
binary_metadata = await data_client.binary_data_by_filter(
binary_metadata, count, last = await data_client.binary_data_by_filter(

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work. It produces this error:

  File "/Users/sierraguequierre/Desktop/data-client-qa.py", line 394, in main
    binary_metadata, count, last = await data_client.binary_data_by_filter(
                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: DataClient.binary_data_by_filter() got an unexpected keyword argument 'include_file_data

the type returned is a list. I don't know why the error is like that but the way I have it does work and does not produce that error. Should I file a bug/are you able to reproduce? @npentrel

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for context this is the extent of my code that produces that error:
binary_metadata, count, last = await data_client.binary_data_by_filter( include_file_data=False )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually nvm just tested it again and it doesn't now. no idea why that was happening but can't repro. weird

)

my_ids = []

for obj in binary_metadata:
for obj in binary_metadata[0]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for obj in binary_metadata[0]:
for obj in binary_metadata:

static/include/app/apis/generated/data.md Show resolved Hide resolved
)


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@viambot
Copy link
Member

viambot commented Sep 19, 2024

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/3457

@sguequierre sguequierre merged commit 0fcaa11 into viamrobotics:main Sep 19, 2024
9 checks passed
@sguequierre sguequierre deleted the DOCS-2899/data-client-qa branch September 19, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants