search

applied-bioinformatics / An-Introduction-To-Applied-Bioinformatics

Interactive lessons in bioinformatics.
http://readIAB.org
Other
805 stars 316 forks source link

very minor suggestions to database-searching.md #290

Closed AstrobioMike closed 6 years ago

AstrobioMike commented 6 years ago

In reference to JOSE review: https://github.com/openjournals/jose-reviews/issues/27

Nothing suggested is considered required for me to support publication

gregcaporaso commented 6 years ago

Thanks for these PRs @AstrobioMike! This travis failure is strange - were you able to run the notebook locally, or are you working only with the markdown or static versions of the content? At first I was thinking the failures were unrelated to your pull requests, but it looks like that might not be the case.

gregcaporaso commented 6 years ago

@AstrobioMike, regarding your move of all of the imports to the top: I get the motivation, though in a lot of cases you still will need to re-execute all of the above cells to pick up where you left off (because variables are often defined early on that are then used later). I think it's probably a good idea to bump all of the imports to the top, but if I do that I'd like to do a sweep through all chapters and make that change at once for consistency. Would you mind reverting the commits that made this change in both of your PRs, and instead creating an issue to make this change to all chapters (or I can create the issue if you prefer).

AstrobioMike commented 6 years ago

Sure thing! Awesome resource and really great work on it.

Regarding the error, sorry for the complication. I ended up doing basically the worst thing possible, of course. I was sticking to the install instructions and stuff playing like a learner/reviewer rather than a contributor. So I was running the notebook I downloaded from the install page locally and then making suggested edits on a totally separate one in github that I couldn't see if I was breaking anything.

It'll be easy for me to launch a cloned one and incorporate my suggestions more cleanly anyway and then I'll submit new PRs.

Regarding the importing all functions at the top, that makes sense. I figured there could be many reasons it wouldn't work that I just wouldn't be aware of until trying it everywhere and running into a problem (already I saw some duplicate functions getting overwritten that made me re-insert an import later). I won't include them in the new PRs I submit.

Great resource!

gregcaporaso commented 6 years ago

All sounds good, thanks so much for be willing to contribute and test! Please let me know if you need any help with the contribution - it'll be great to get all of your fixes in!