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

Dask search cv #98

Closed
wants to merge 0 commits into from
Closed

Conversation

rdvelazquez
Copy link
Member

This notebook shows the implementation of dask-searchcv to speed up the pipeline and allow for implementing PCA in cross-validation. This PR is the first step in addressing Issue #94 ... The next step will be to implement dask-searchcv in the main notebooks after #93 is merged.

@dhimmel: I'm not sure why this is showing 10 commits (it's including the commits from PR #95). This may have something to do with the fact that I started #95 by just uploading files on github.com and/or that I was accidentally using my master branch. Let me know if you have any advice or input on that. Sorry about my lack of git experience (I think I'm finally getting it now).

@dhimmel
Copy link
Member

dhimmel commented Jun 6, 2017

I'm not sure why this is showing 10 commits

Can you rebase on the current version of cognoma:master?

git fetch upstream
git rebase upstream/master

This will replay your commits on top of the latest cognoma commits and remove the commits that are essentially duplicates.

Then you will have to git push --force.

@rdvelazquez
Copy link
Member Author

@dhimmel Thank you for the pointers! What you recommended makes sense but I'm still having some trouble rebasing. I suspect the issues are associated with the two errors I made with my original pca pull request: (1) editing and pull requesting from my master branch and (2) deleting and re-uploading files with the same filenames.

Would starting from scratch be an option? i.e. deleting my forked and local repos, re-forking and cloning the cognoma/machine-learning repo and starting a new (correct) pull request for this daskSearchCV notebook?

If that isn't an option and you'd prefer me to rebase, I'll try to figure out the issues I'm having and I can post my error messages/get some help from you if I can't figure out how to solve the issues with online resources.

@dhimmel
Copy link
Member

dhimmel commented Jun 7, 2017

Would starting from scratch be an option?

Perhaps, but maybe a bit extreme. I think you should be able to fix it on a branch by branch basis.

On your master branch you can do:

git fetch upstream
git reset --hard upstream/master

This should make your master branch equal cognoma:master (as we want).

I'm still having some trouble rebasing.

What issues are you having? Since you're rebasing on upstream/master, your local master branch should not matter for this operation.

@rdvelazquez
Copy link
Member Author

So I reset my master branch; it's now even with cognoma:master. However when I reset my dask-searchCV branch it not only removed the duplicate commits but also the commits where I uploaded the dask-searchCV notebook (so the dask-searchCV branch was then also even with cognoma:master), which seems to have closed this pull request.

I re-added the dask-searchCV notebook (and .py file) to my dask-searchCV branch, thinking it might automatically re-open this pull request but it didn't. I think if I pull request the dask-searchCV branch again it may open up a new pull request and I think we want to keep this under this PR (#98).

Any advice on how to proceed. Thank you so much for the help and for working with me!

What issues are you having? Since you're rebasing on upstream/master, your local master branch should not matter for this operation.

Based on my limited knowledge of git and what I was looking up online, I was getting merge conflicts for duplicate files. The advice I found online (stack overflow and the like) was to use git checkout --theirs (filename) to deal with the merge conflict (essentially telling git to just use the cognoma:master file in cases of conflict but that didn't seem to completely remedy the situation. I don't still have the error messages from those attempts (I wish I had saved them so I could post for your reference).

@dhimmel
Copy link
Member

dhimmel commented Jun 7, 2017

I think if I pull request the dask-searchCV branch again it may open up a new pull request and I think we want to keep this under this PR (#98).

Most likely. Don't worry about opening a new PR. Just in that PR, mention that it continues on #98.

@rdvelazquez
Copy link
Member Author

Will do. Thanks!

Remember that time I said "Sorry about my lack of git experience (I think I'm finally getting it now)." and then proceeded to basically break git... 😮

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.

2 participants