Skip to content

[SN-147] Created quick start guide to geared to new users #1640

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

Merged
merged 17 commits into from
May 31, 2024

Conversation

Gabefire
Copy link
Collaborator

No description provided.

@Gabefire Gabefire requested a review from a team as a code owner May 29, 2024 15:48
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Gabefire Gabefire force-pushed the gu/nb_quickstart branch from b3846e9 to 620c733 Compare May 29, 2024 18:55
@Gabefire Gabefire requested a review from paulnoirel May 29, 2024 19:55
@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 30, 2024

Choose a reason for hiding this comment

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

Prefer shell commands (!) over magic commands (%).


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Magic installs it to the correct env my linter goes nuts over that actually lol

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 30, 2024

Choose a reason for hiding this comment

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

Line #2.    

Don't use the default integration if not relevant

dataset = client.create_dataset(name="Quick Start Example Dataset", iam_integration=None)

Reply via ReviewNB

@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 30, 2024

Choose a reason for hiding this comment

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

Suggestion: Use export_v2() instead which is easier to grasp.


Reply via ReviewNB

Copy link
Collaborator Author

@Gabefire Gabefire May 30, 2024

Choose a reason for hiding this comment

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

Engineering has repeatedly said the long-term plan is to remove export_v2. (no exact plans yet) Because of this, I want to get people to use to Export. Fewer people on export_v2 == fewer people causing chaos once they make the decision. Also less confusing overall since export seems like V1 lol and its not. Moving docs to that direction right now will cause less work in the future

@Gabefire Gabefire requested a review from paulnoirel May 30, 2024 14:36
@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 31, 2024

Choose a reason for hiding this comment

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

minor one: "our SDK" -> "the Labelbox SDK"


Reply via ReviewNB

@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 31, 2024

Choose a reason for hiding this comment

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

"install the labelbox library" -> "install the labelbox library"


Reply via ReviewNB

@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 31, 2024

Choose a reason for hiding this comment

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

Same as earlier.


Reply via ReviewNB

@@ -0,0 +1,195 @@
{
Copy link
Contributor

@paulnoirel paulnoirel May 31, 2024

Choose a reason for hiding this comment

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

I think you can remove the comment "specify the media type" since it doesn't add much.


Reply via ReviewNB

paulnoirel
paulnoirel previously approved these changes May 31, 2024
Copy link
Contributor

@paulnoirel paulnoirel left a comment

Choose a reason for hiding this comment

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

LGTM

@Gabefire Gabefire requested a review from paultancre May 31, 2024 15:03
Copy link
Contributor

@paultancre paultancre left a comment

Choose a reason for hiding this comment

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

LGTM

@Gabefire Gabefire merged commit b142a6f into develop May 31, 2024
4 checks passed
@Gabefire Gabefire deleted the gu/nb_quickstart branch May 31, 2024 15:04
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.

3 participants