-
Notifications
You must be signed in to change notification settings - Fork 80
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
connecting tutorials to CMI #2130
connecting tutorials to CMI #2130
Conversation
This is ready for review but I think that after reviews (when ready to merge) it will be worth doing a final change: moving the folder tutorials to guides. I didn't do this cause review will be much harder. cc: @tkosciol |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall.
I posted some minor comments.
@@ -1,64 +0,0 @@ | |||
.. _account-creation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMI workshop doc (http://cmi-workshop.readthedocs.io/en/latest/qiita-create-study.html#signing-up-for-a-qiita-account) is not as comprehensive as this one.
Do you think that we should get rid of retrieving password info, etc.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to improve the documentation that is in the CMI workshop ... it could be a copy/paste of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! Working on it on https://github.com/biocore/cmi-workshops side. Will ask you for review.
template and a single prep template. | ||
|
||
However, Qiita can also support more complex study designs. For example | ||
imagine a study with 100 samples in which: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMI workshop does not cover those complex designs so far. But there is meta-analysis section (http://cmi-workshop.readthedocs.io/en/latest/qiita-16S-analysis.html#creating-a-meta-analysis).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same than before or I can see an argument of leaving this one here? Up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we don't have it in CMI workshop tutorial is that we don't use it with the test data that we provide, i.e. the idea is to show each step with example data.
I'd be happy to move it to CMI tutorial if/when we have a more complex dataset for people to work on.
In the current format, I'd rather not complicate the dataset, but we were discussing "intermediate" or "advanced" tutorial and that would fit perfectly there.
tl;dr: let's keep it in Qiita documentation for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, will add it to the philosophy page ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tkosciol ... I just added it, had to make some changes so it fits better in that page
takes a couple of days but can take more depending on availability and how busy | ||
is the submitting queue. | ||
|
||
Study status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sandbox etc. is not covered by the CMI workshop tutorial.
Should we maybe move it to Qiita philosophy/documentation part? i.e. keep it, but in a different place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable. However, I think Figure 2 of https://qiita.ucsd.edu/static/doc/html/qiita-philosophy/index.html covers it, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree. I missed that Figure previously.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tkosciol thanks for the detailed review ...
@@ -1,64 +0,0 @@ | |||
.. _account-creation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to improve the documentation that is in the CMI workshop ... it could be a copy/paste of this.
template and a single prep template. | ||
|
||
However, Qiita can also support more complex study designs. For example | ||
imagine a study with 100 samples in which: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same than before or I can see an argument of leaving this one here? Up to you.
takes a couple of days but can take more depending on availability and how busy | ||
is the submitting queue. | ||
|
||
Study status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable. However, I think Figure 2 of https://qiita.ucsd.edu/static/doc/html/qiita-philosophy/index.html covers it, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go, provided that comments are addressed.
Ping me when ready to merge.
takes a couple of days but can take more depending on availability and how busy | ||
is the submitting queue. | ||
|
||
Study status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree. I missed that Figure previously.
@@ -1,64 +0,0 @@ | |||
.. _account-creation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! Working on it on https://github.com/biocore/cmi-workshops side. Will ask you for review.
template and a single prep template. | ||
|
||
However, Qiita can also support more complex study designs. For example | ||
imagine a study with 100 samples in which: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we don't have it in CMI workshop tutorial is that we don't use it with the test data that we provide, i.e. the idea is to show each step with example data.
I'd be happy to move it to CMI tutorial if/when we have a more complex dataset for people to work on.
In the current format, I'd rather not complicate the dataset, but we were discussing "intermediate" or "advanced" tutorial and that would fit perfectly there.
tl;dr: let's keep it in Qiita documentation for now.
cc-ing: @adswafford |
ping @tkosciol and @adswafford |
Sorry! Will review before the end of the day. |
Don't know the etiquette, and can't find any buttons to click to approve, but the changes look good to me. Maybe I can get a quick tutorial on this tomorrow? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @antgonza !
However, I don't have permissions to merge.
@tkosciol, you should have permissions now ... thanks! |
forgot to say: @adswafford, let me know if you want to chat, just send me an email ... |
No description provided.