Skip to content

Conversation

karenkyang
Copy link
Contributor

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -3,7 +3,9 @@
{
Copy link
Contributor

@jtsodapop jtsodapop May 27, 2022

Choose a reason for hiding this comment

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

Do we want to expose all of this? It seems like a lot of text that isn't relevant to the user. Maybe just show only one, similar to the below sections?


Reply via ReviewNB

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 will clear the text output of this cell. I do want to tell users that they can list all available metadata fields in SDK

@@ -3,7 +3,9 @@
{
Copy link
Contributor

@jtsodapop jtsodapop May 27, 2022

Choose a reason for hiding this comment

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

Can we change this structure slightly to clearly show we have 2 options as listed below?

  • Option 1: .....
  • Option 2: ....

Reply via ReviewNB

@@ -3,7 +3,9 @@
{
Copy link
Contributor

@jtsodapop jtsodapop May 27, 2022

Choose a reason for hiding this comment

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

Unsure if we want to expose all of custommetadatatime as it does take up a lot of space


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Will only print out metadata info

@@ -3,7 +3,9 @@
{
Copy link
Contributor

@jtsodapop jtsodapop May 27, 2022

Choose a reason for hiding this comment

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

similar to my comment above


Reply via ReviewNB

@@ -3,7 +3,9 @@
{
Copy link
Contributor

@jtsodapop jtsodapop May 27, 2022

Choose a reason for hiding this comment

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

I think one of our initiatives from before was to remove empty spaces like this


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah ... I am okay with leaving it for now.

it will contain error messages if there is error occurs?

@karenkyang karenkyang requested a review from jtsodapop May 27, 2022 17:12
@karenkyang karenkyang merged commit f5173ae into develop May 27, 2022
@jtsodapop jtsodapop deleted the metadata_tutorial_update branch September 14, 2022 15:52
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.

2 participants