Skip to content
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

FIX: Update and sync DTI episode #116

Merged
merged 7 commits into from
Apr 5, 2021

Conversation

kaitj
Copy link
Collaborator

@kaitj kaitj commented Apr 2, 2021

This notebook clears the outputs of the DTI solutions notebook. It also fixes the links to static images within both solutions and non-sollutions notebook.

@kaitj kaitj requested a review from jhlegarreta April 2, 2021 13:29
@netlify
Copy link

netlify bot commented Apr 2, 2021

Deploy preview for carpentries-dmri ready!

Built with commit 1e5254f

https://deploy-preview-116--carpentries-dmri.netlify.app

@kaitj kaitj changed the title FIX: Clear notebook output, fix image links FIX: Clear DTI notebook output, fix image links Apr 2, 2021
@kaitj kaitj added the type:bug Code included in the lesson needs to be fixed label Apr 2, 2021
@jhlegarreta
Copy link
Contributor

jhlegarreta commented Apr 3, 2021

On a slightly related note:

Definitely they need to be synchronized (paying attention to the relative folder level when trying to look for the data).

@kaitj
Copy link
Collaborator Author

kaitj commented Apr 3, 2021

I have updating all of the jupyter notebooks (aside from the intro at the moment) on my to do list - waiting to finalize all of the exercises before doing so!

@kaitj kaitj self-assigned this Apr 5, 2021
@kaitj kaitj added the status:in progress Contributor working on issue label Apr 5, 2021
@kaitj kaitj marked this pull request as draft April 5, 2021 15:46
@kaitj
Copy link
Collaborator Author

kaitj commented Apr 5, 2021

A few updates since last review! Started with this one as it was the easiest of the 3 notebooks I'm working on updating.

  1. Branch has been rebased from master.
  2. Tried to fix (as best as I could) the characters per line in the markdown file
  3. Synced markdown to both notebooks and updated the paths as needed - hopefully I didn't miss anything here.
  4. Cleared output and remove solutions from non-solutions notebook.
  5. Ran autopep8 over the jupyter notebooks, so should be pep8 compliant for the code cells.

It should be grabbing the subject at the beginning of the notebook (exercise not withstanding), let me know if I missed a cell!

@kaitj kaitj marked this pull request as ready for review April 5, 2021 21:51
@kaitj kaitj changed the title FIX: Clear DTI notebook output, fix image links FIX: Update and sync DTI episode Apr 5, 2021
@kaitj kaitj requested a review from josephmje April 5, 2021 22:03
Copy link
Contributor

@jhlegarreta jhlegarreta left a comment

Choose a reason for hiding this comment

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

Have not tried locally.

An inline comment. Otherwise looks good at first sight.

@kaitj
Copy link
Collaborator Author

kaitj commented Apr 5, 2021

Related to #100 - takes care of the DTI notebooks portion

@kaitj kaitj merged commit 0fa41f5 into carpentries-incubator:master Apr 5, 2021
@kaitj kaitj deleted the update_dti branch April 5, 2021 23:24
@kaitj kaitj removed the status:in progress Contributor working on issue label Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Code included in the lesson needs to be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants