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.

Sign up for GitHub

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

Jump to bottom

Add exercises questions to merged chapters #46

Merged
merged 7 commits into from
Sep 2, 2019
Merged

Add exercises questions to merged chapters #46

merged 7 commits into from
Sep 2, 2019

Conversation

AlexAndorra
Copy link

As discussed in #40 here are the already merged notebooks, where I added the questions before the answers. The notebooks should be even easier to follow for readers.

Note that I didn't re-run the notebooks - under the assumption that they run correctly because they were already merged - but can do it if needed.

Finally, in chapter 3, I noticed that :

  • the answer to question 8 does not seem complete
  • cells for questions 13 to 15 have not been run

Tell me if you need help with that @canyon289 😉
Take care and PyMCheers!

@canyon289
Copy link
Collaborator

Thanks @AlexAndorra
I'll fix Chapter 3. I think this is based off an older commit than master though :( I merged the Chapter 3 and 4 notebooks which I believe is causing the merge conflict.

@AlexAndorra
Copy link
Author

AlexAndorra commented Aug 24, 2019 via email

@canyon289
Copy link
Collaborator

canyon289 commented Aug 25, 2019 via email

@AlexAndorra
Copy link
Author

Just did a git fetch upstream and no other commits get pulled down 😞
I remember you merged chapters 3 and 4 after I had begun my work on branch add-questions-to-merged-nbs, so I fetched the changes, integrated the new nbs into this branch, and added the questions directly on them. Do you see where this could be a problem?

@canyon289
Copy link
Collaborator

From what you described things should be fine. I'm just not sure how a merge conflict occurred. No worries though, hopefully they aren't too bad to resolve. I'll hold off on making any changes until this branch is merged.

@aloctavodia
Copy link
Owner

Hi, sorry for being late to the party. I see there are conflicts but I am not able to solve see them or solve them using github. @AlexAndorra did you follow these recommendations?

Step 1: From your project repository, check out a new branch and test the changes.

git checkout -b AlexAndorra-add-questions-to-merged-nbs master
git pull https://github.com/AlexAndorra/BAP.git add-questions-to-merged-nbs

Step 2: Merge the changes and update on GitHub.

git checkout master
git merge --no-ff AlexAndorra-add-questions-to-merged-nbs
git push origin master

@AlexAndorra
Copy link
Author

Mmmh seems like I missed a couple steps there...

When started working on this feature, I did git checkout -b add-questions-to-merged-nbs, modified the notebooks, added complete chapters 3 and 4 when Ravin merged them to upstream/master, pushed all of that to origin/add-questions-to-merged-nbs, and then did the pull request as per this procedure.

Can I do what you recommend or do we need something else?

@aloctavodia
Copy link
Owner

Those steps should fix this (hopefully)

@AlexAndorra
Copy link
Author

Ok, so I can't do the first step (git pull https://github.com/AlexAndorra/BAP.git add-questions-to-merged-nbs) because there is a conflict:

(stat-rethink-pymc3) MBPdealeandorra:BAP alexandreandorra$ git checkout -b AlexAndorra-add-questions-to-merged-nbs master
Switched to a new branch 'AlexAndorra-add-questions-to-merged-nbs'

(stat-rethink-pymc3) MBPdealeandorra:BAP alexandreandorra$ git pull https://github.com/AlexAndorra/BAP.git add-questions-to-merged-nbs
From https://github.com/AlexAndorra/BAP
 * branch            add-questions-to-merged-nbs -> FETCH_HEAD
Auto-merging exercises/Chapter3.ipynb
CONFLICT (add/add): Merge conflict in exercises/Chapter3.ipynb
Automatic merge failed; fix conflicts and then commit the result.
Error redoing merge 1b3d53ef8796fdd287d28fc6449af86ae05ee246
(stat-rethink-pymc3) MBPdealeandorra:BAP alexandreandorra$ git status
HEAD detached from refs/heads/AlexAndorra-add-questions-to-merged-nbs
You have unmerged paths.
  (fix conflicts and run "git commit")
  (use "git merge --abort" to abort the merge)

Changes to be committed:

	new file:   exercises/Chapter3_Part2.ipynb

Unmerged paths:
  (use "git add <file>..." to mark resolution)

	both added:      exercises/Chapter3.ipynb

The problem is I can't open the Chapter 3 NB in jupyter to see the conflicts and resolve them... Do you where to go from there?
Also, I'm not sure Chapter3_Part2.ipynb should come up here, as Ravin deleted it when he consolidated the notebooks

@aloctavodia
Copy link
Owner

Not sure how to proceed now, @canyon289 o you have any suggestions or hint?

@canyon289
Copy link
Collaborator

There's two paths.
Open the notebooks files in a text editor and fix them manually which could be a huge pain, or just make the changes again in the Chapter 3 and Chapter 4 notebooks

@AlexAndorra
Copy link
Author

Thank you Ravin! So you mean fetch the last changes from upstream/master, create a whole new branch, and re-change the notebooks on this new branch?

@canyon289
Copy link
Collaborator

canyon289 commented Aug 30, 2019 via email

@AlexAndorra
Copy link
Author

I just deleted chapters 3 and 4 from this PR, which solved the conflicts. Will do a whole new PR for 3 and 4 ASAP, as per Ravin advice. BTW, thanks for your help Ravin, taught me valuable git lessons 😉

@AlexAndorra AlexAndorra mentioned this pull request Sep 2, 2019
Merged
@aloctavodia
Copy link
Owner

Great, thank you!

@aloctavodia aloctavodia merged commit 8f139c7 into aloctavodia:master Sep 2, 2019
@canyon289
Copy link
Collaborator

@AlexAndorra No problem. I'm sorry about the merge conflicts, it's truly a pain when it comes to notebooks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexAndorra @canyon289 @aloctavodia