-
Notifications
You must be signed in to change notification settings - Fork 590
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
Update datasets to use API #6126
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6126 +/- ##
==========================================
+ Coverage 99.58% 99.69% +0.11%
==========================================
Files 443 444 +1
Lines 42273 42141 -132
==========================================
- Hits 42096 42012 -84
+ Misses 177 129 -48 ☔ View full report in Codecov by Sentry. |
….com:PennyLaneAI/pennylane into sc-70918-pennylane-oss-uses-new-datasets-api
|
||
GRAPHQL_URL = "https://cloud.pennylane.ai/graphql" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GRAPHQL_URL = "https://cloud.pennylane.ai/graphql" | |
import os | |
GRAPHQL_URL = os.getenv("DATASETS_ENDPOINT_URL", "https://cloud.pennylane.ai/graphql") |
It'll be useful to be able to override this for testing new datasets
branch = parameter_tree | ||
choices = [] | ||
for param in parameters: | ||
print(f"Available options for {param}:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We should move the leaf check to the top of the loop, this is helpful because it guarantees that
branch
is always a non-leaf node in the rest of the loop - A leaf node is a
str
, not adict
print(f"Available options for {param}:") | |
if isinstance(branch, str): | |
break | |
if len(branch["next"]) == 1: | |
branch = next(iter(branch["next"].values())) | |
continue | |
print(f"Available options for {param}:") |
branch = branch["next"][user_value] | ||
except KeyError as e: | ||
raise ValueError(f"Must enter a valid {param}:") from e | ||
choices.append(user_value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
choices.append(user_value) |
I don't think we need choices
since we're not returning it
if "next" in branch: | ||
if len(branch["next"]) == 1: | ||
branch = next(iter(branch["next"].values())) | ||
continue | ||
branch = branch["value"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if "next" in branch: | |
if len(branch["next"]) == 1: | |
branch = next(iter(branch["next"].values())) | |
continue | |
branch = branch["value"] |
Since we moved this logic to the top of the loop
"""Prompts the user to select parameters for datasets one at a time.""" | ||
|
||
branch = parameter_tree | ||
choices = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
choices = [] |
Context:
Datasets are currently downloaded by hitting the S3 bucket (via CloudFront) where the
.h5
files are stored. This PR updates the way datasets are downloaded by directing download requests to the Software Cloud managed Datasets Service.Description of the Change:
The
qml.data.load
function now queries the Datasets API in order to download datasets.Benefits:
foldermap
anddata_struct
files, alleviating the need to manually manage them.Possible Drawbacks:
Related GitHub Issues: