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

Minor update to dataset docs to lead users to quickstart and list_datasets #5484

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

DSGuala
Copy link
Contributor

@DSGuala DSGuala commented Apr 9, 2024

Context:
Users have trouble finding all bondlength and basis combinations for qchem molecules. They end up using the slider on the website to check which bond lengths are available one-by-one.
image

Description of the Change:

  • Updated the quickstart guide to say "viewing available datasets" instead of "dataset structure"
  • Updated the qml.data docs to encourage users to first read the quickstart guide

Benefits:
Users should be more likely to reach the list_datasets description in the quickstart guide and know that it can be used to view the available datasets.

Possible Drawbacks:
Users who use pennylane.ai/datasets but don't go to the documentation still won't see this.

Copy link
Contributor

github-actions bot commented Apr 9, 2024

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@trbromley
Copy link
Contributor

@DSGuala is this needed for the v0.36 release?

@DSGuala
Copy link
Contributor Author

DSGuala commented Apr 26, 2024

@DSGuala is this needed for the v0.36 release?

@trbromley Not required. We can leave it for the v0.37 release.

@josh146
Copy link
Member

josh146 commented Apr 26, 2024

@DSGuala just to clarify, is this PR complete or did you still have changes you want to make? Because if it is complete, we could merge during feature freeze next week

@DSGuala
Copy link
Contributor Author

DSGuala commented Apr 30, 2024

@DSGuala just to clarify, is this PR complete or did you still have changes you want to make? Because if it is complete, we could merge during feature freeze next week

@josh146 this is basically completed. But I haven't had the bandwidth to take a final look or get it reviewed.

@trbromley
Copy link
Contributor

@DSGuala would it make sense to come back to this now and get it merged in soon?

@DSGuala
Copy link
Contributor Author

DSGuala commented May 24, 2024

@DSGuala would it make sense to come back to this now and get it merged in soon?

Yup! I will take a look today.

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Approved this PR @DSGuala! Though let me know if you'd like to change anything and I'll take another look.

@DSGuala DSGuala added this to the v0.37 milestone Jun 19, 2024
@DSGuala DSGuala marked this pull request as ready for review June 19, 2024 18:46
Copy link
Contributor

@obliviateandsurrender obliviateandsurrender left a comment

Choose a reason for hiding this comment

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

Looks good! Just one optional suggestion.

Users who use pennylane.ai/datasets but don't go to the documentation still won't see this.

Regarding the drawback, a couple of suggestions:

  1. We can link the quick start page in the download dialogue box as hyperlinked via ? or More details. Otherwise, you may add a code snippet to know more about that particular dataset using list_dataset. (the second option might be uglier)
  2. We should arrange a centralized tabulated- or file-explorer-like page to showcase a snapshot of all datasets we host based on the website. You may think of it as a prettier version of list_datasets, which we can simply link to.

doc/introduction/data.rst Outdated Show resolved Hide resolved
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.66%. Comparing base (66421f1) to head (dac19a2).
Report is 242 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5484      +/-   ##
==========================================
- Coverage   99.67%   99.66%   -0.01%     
==========================================
  Files         422      422              
  Lines       40658    40362     -296     
==========================================
- Hits        40525    40228     -297     
- Misses        133      134       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DSGuala DSGuala merged commit 14a0b63 into master Jun 20, 2024
40 checks passed
@DSGuala DSGuala deleted the update_datasets_docs branch June 20, 2024 15:33
mudit2812 pushed a commit that referenced this pull request Jul 2, 2024
…asets (#5484)

**Context:**
Users have trouble finding all `bondlength` and `basis` combinations for
qchem molecules. They end up using the slider on the website to check
which bond lengths are available one-by-one.

![image](https://github.com/PennyLaneAI/pennylane/assets/67476785/fe2686b9-8f37-4e4e-854f-bf4708e0900a)


**Description of the Change:**
- Updated the quickstart guide to say "viewing available datasets"
instead of "dataset structure"
- Updated the `qml.data` docs to encourage users to first read the
quickstart guide

**Benefits:**
Users should be more likely to reach the `list_datasets` description in
the quickstart guide and know that it can be used to view the available
datasets.

**Possible Drawbacks:**
Users who use pennylane.ai/datasets but don't go to the documentation
still won't see this.

---------

Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
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