Skip to content

Conversation

@denbonte
Copy link
Contributor

@denbonte denbonte commented Sep 1, 2023

As per discussion

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Gibbsdavidl
Copy link

Looks good and all works for me.

A couple small comments:

  • I think I would make the demo project ID a nonsense string.
  • Maybe want to explain why we would keep only the trackingIdentifier classes with more than 100 entries in the dataframe.
  • Could name the y-axis label "Volume" and the title "Boxplot of Volumes by Tracking Identifier"
  • Could add a conclusion that tells people where to go next. ?

@fedorov
Copy link
Member

fedorov commented Sep 20, 2023

@denbonte can you please respond to @Gibbsdavidl comments? Thank you for the review, Dave!

@denbonte
Copy link
Contributor Author

Hey All,

Thanks for the feedback @Gibbsdavidl 🙃

A couple small comments:

  • I think I would make the demo project ID a nonsense string.

Agreed - I changed this to a random string in the revised version.

  • Maybe want to explain why we would keep only the trackingIdentifier classes with more than 100 entries in the dataframe.

I tried to make it a bit explicit it was for visualization reasons!

  • Could name the y-axis label "Volume" and the title "Boxplot of Volumes by Tracking Identifier"

Done!

  • Could add a conclusion that tells people where to go next. ?

Done - I added a snippet on the IDC documentation page on files and metadata and some text!

Let me know if it looks good - and feel free to merge if yes!

@fedorov
Copy link
Member

fedorov commented Sep 29, 2023

@denbonte I am sorry for not noticing this earlier, but I think we also should add a preamble, and point users to the getting started part 1 tutorial that will show them how to set up their own project. I will merge and add that myself. Thank you!

@fedorov fedorov merged commit 2e98432 into ImagingDataCommons:master Sep 29, 2023
@denbonte denbonte deleted the colab-r branch October 1, 2023 03:09
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