Closed ammedmar closed 4 years ago
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.
:white_check_mark: ulupo
:x: ammedmar
@ammedmar had to take care of several merge conflicts arising from having recently merged #363, which would have been a little prohibitive to explain for you to do yourself. But let me update you on a few important changes introduced in #363 due to constraints imposed by the system we are using to render on the web:
I hope my resolution of the conflicts was good. I was as careful as possible but please check.
A note: I noticed the section on L^p norms is under construction.
I see, I understand the changes to follow in the future. I am confused about the fact that it seems you merged master into vectorization but the PR is still open. Should I pull from master to get your changes?
I am confused about the fact that it seems you merged master into vectorization but the PR is still open.
That's just what Git is calling me resolving the merge conflicts. Merging your branch into master is what would be at odds with the PR remaining open.
OK, so trying to be more prescriptive. Should I pull from gtda master, resolve the conflicts with my vectorization branch and update this PR?
@ammedmar, pulling from master is almost never a bad idea. But I did that for you: resolving merge conflicts means in particular merging master into your branch, so you should see nothing new and no merge conflicts. You can check exactly how things were changed by my intervention in https://github.com/ammedmar/giotto-tda/commit/4062ff17cb3c8405d38c8c4ab8ad0c906fa48c5c#diff-ff0931b0232e6abc20b3da4a6a943da4 (click on "load diff" as the diff is particularly large for the glossary)
Reference issues/PRs Fixes #361
Description Adds vectorization, kernel and amplitude entry, and modifies P. landscapes and P. silhouettes to match the terminology. Other consistency updates are also included.
Any other comments? An entry for L^p spaces is expected and will be included with the entry on P landscapes given their tight connection.