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

[SYNPY-1384] Create uploading data in bulk tutorial #1101

Merged
merged 4 commits into from
May 30, 2024

Conversation

BryanFauble
Copy link
Contributor

@BryanFauble BryanFauble commented May 29, 2024

Problem:

  1. We had no uploading data in bulk tutorial
  2. Some of the tutorials used a directory structure that did not match the flattened layout
  3. Some install instructions were confusing and led to some folks having issues as they ran what we had provided

Solution:

  1. Creating the uploading data in bulk tutorial
  2. Revamp some tutorials to follow the flattened data layout
  3. Updated install commands
  4. Removed some copy/pasted scripts. Instead dynamically showing specific lines of python in the tutorial

Testing:

  1. I stepped through all of the tutorials in order and verified that I was able to run them with the expected outcome.
  2. Verified I could run the install commands as provided.

@BryanFauble BryanFauble requested a review from a team as a code owner May 29, 2024 22:20
Copy link
Contributor

@BWMac BWMac left a comment

Choose a reason for hiding this comment

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

LGTM! I followed along and went through the updated docs and everything looks good and worked as expected.

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 Thanks for the review and work! Just tiny comment.


print(f"Batch 1 Folder ID: {batch_1_folder_id}")

{!docs/tutorials/python/tutorial_scripts/annotation.py!lines=5-22}
Copy link
Member

Choose a reason for hiding this comment

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

This is neat, although does this put a risk on making sure we don't notify these tutorial_scripts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree there is some risk here. My opinion is its a risk by manually copying the code over. At least in this way the code gets updated and the lines for each step is updated. I guess it'll be something to look out for in future PRs if there are changes.

Co-authored-by: Brad Macdonald <52762200+BWMac@users.noreply.github.com>
@BryanFauble BryanFauble merged commit 7b98106 into develop May 30, 2024
16 checks passed
@BryanFauble BryanFauble deleted the SYNPY-1384-uploading-data-in-bulk branch May 30, 2024 21:29
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.

None yet

3 participants