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

Projection to closed curves #1015

Closed
wants to merge 254 commits into from

Conversation

Florent-Michel
Copy link
Contributor

Addition of classes for closed curves with a projection method.

@ninamiolane
Copy link
Collaborator

@Florent-Michel do you have updates on this? :D

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Florent-Michel
Copy link
Contributor Author

Florent-Michel commented Jul 8, 2021

Hi @ninamiolane, sorry for the delay, I just wrote the unit tests related to this PR. However, when I tried to update my branch with git rebase upstream/master as said in the "Contributing" page, it created a lot of commits, which made the PR a bit messy.

nguigs added a commit that referenced this pull request Jul 14, 2021
Advise to merge instead of rebase when it's been a while, to avoid the situation of #1015
@nguigs nguigs mentioned this pull request Jul 14, 2021
ninamiolane pushed a commit that referenced this pull request Jul 14, 2021
Advise to merge instead of rebase when it's been a while, to avoid the situation of #1015
@nguigs
Copy link
Collaborator

nguigs commented Jul 14, 2021

Hi @ninamiolane, sorry for the delay, I just wrote the unit tests related to this PR. However, when I tried to update my branch with git rebase upstream/master as said in the "Contributing" page, it created a lot of commits, which made the PR a bit messy.

Hi @Florent-Michel, I've created a new branch and cherry-picked your commits only so that we can review your PR properly in #1053. If you want to edit it, you can pull the branch with

git checkout -b Florent-curves-new master
git pull https://github.com/nguigs/geomstats.git florent-curves-clean

and then open a new PR.
If something is missing on the new branch, let me know.

@nguigs nguigs closed this Jul 14, 2021
@Florent-Michel
Copy link
Contributor Author

Thanks @nguigs, I pulled the branch, edited it and opened a new PR.

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.

None yet

7 participants