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

updated API_KEY to ROBOFLOW_API_KEY for clarity #202

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

josephofiowa
Copy link
Contributor

Description

I updated the gaze_detection example such that the API_KEY is named as ROBOFLOW_API_KEY as this is what the examples in the docs (https://inference.roboflow.com/foundation/gaze/#how-to-use-l2cs-net and https://github.com/roboflow/inference/tree/main/examples/gaze-detection) name the environment variable for the Roboflow API key

Type of change

Please delete options that are not relevant.

  • [ X ] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

Yes, I've tried this change locally.

Any specific deployment considerations

None

Docs

  • Docs updated? What were the changes:

This change makes the gaze_detection example work with the docs as-is.

@CLAassistant
Copy link

CLAassistant commented Dec 20, 2023

CLA assistant check
All committers have signed the CLA.


```bash
python3 -m venv venv
source venv/bin/activate
pip install -r requirements.txt
pip install -r ../../requirements/_requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not tested that but I was always thinking that for gaze to work we need also ../../requirements/requirements.gaze.txt - at least this is suggested by setup.py extras that we created to build libs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, only these are required in gaze.py:

import base64
import cv2
import numpy as np
import requests
import os

We can assume the user is installing inference, overall, and have them setup the base _requirements.txt. In the future, we can decide to scope to individual examples.

@@ -6,10 +6,10 @@
import os

IMG_PATH = "image.jpg"
API_KEY = os.environ["API_KEY"]
ROBOFLOW_API_KEY = os.environ["ROBOFLOW_API_KEY"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This value is fine, as we also allow it in other places as an alternative for API_KEY - in long ran we may decide on single one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's use ROBOFLOW_API_KEY across docs. Good note that the API_KEY = os.environ["API_KEY"]
here needs to be updated: https://inference.roboflow.com/foundation/gaze/#how-to-use-l2cs-net

Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow left a comment

Choose a reason for hiding this comment

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

LGTM with comment about the requirements

@PawelPeczek-Roboflow
Copy link
Collaborator

@paulguerrie - I am not sure why CI fails on the Docker auth

@paulguerrie paulguerrie merged commit f64d58c into roboflow:main Dec 20, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants