[PY] Allow direct import of ripser_parallel and move test folders

[ulupo]

Fixes #15.

In addition, moves test folder containing Python tests inside the python package.

• Update source and test files
• Update notebooks
• Update docs

NOTE: The first commit message lies about what happened to modules, since nothing happened to it.

[MonkeyBreaker]

Nice,
Looking into the discussion in issue #15 we were talking about maybe going into a direction were we would do the following:

from gph.vr import ripser_parallel

Do we drop the vr part ?

[ulupo]

My view is that one does not exclude the other: we can imagine a future in which there are subpackages vr and others, and we can then also allow imports that way. But I think that future is not too close and even when it comes the two import modes can actually coexist! I think Python allows that flexibility.

For now it is nice to be a drop-in replacement to ripser.py where you would do from ripser import ripser.

[MonkeyBreaker]

My view is that one does not exclude the other: we can imagine a future in which there are subpackages vr and others, and we can then also allow imports that way. But I think that future is not too close and even when it comes the two import modes can actually coexist! I think Python allows that flexibility.

Definitely, I updated the documentation, IMO if we did not break anything then we can merge. After the merge is done I will launch the github pages action in order to have the latest commit.

[MonkeyBreaker]

Wait, the example is not updated or do I not see the change ?

[MonkeyBreaker]

About the test I am not sure I would have place them inside the python directory. Because those test are here for validate the repository.

I think we are not in the same situation as with giotto-tda were we have tests inside the python directory that test directly the modules there. And we have another test directory in gtda. Where there we test the functionalities of the entire library. Do you see what I mean ?

[ulupo]

Wait, the example is not updated or do I not see the change ?

You're right? I swear I did this, it must have got lost somewhere.

[ulupo]

@MonkeyBreaker I'm not sure I agree with the view on tests. The tests we currently have are python script tests and test only the bindings, we could imagine (and I think we should hope for!) a future in which C++ developers work on the C++ backend and help up develop tests for those (a bit like happens in GUDHI), separately from the Python integration. Then it would be a bit undesirable to have all tests (Python and not) in the same top-level folder.

To summarize: in my view, giotto-ph is a C++ library as much as it is a Python one, so the two languages should be treated equally as much as we can.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[ulupo]

Wait, the example is not updated or do I not see the change ?

Done now.

[MonkeyBreaker]

@MonkeyBreaker I'm not sure I agree with the view on tests. The tests we currently have are python script tests and test only the bindings, we could imagine (and I think we should hope for!) a future in which C++ developers work on the C++ backend and help up develop tests for those (a bit like happens in GUDHI), separately from the Python integration. Then it would be a bit undesirable to have all tests (Python and not) in the same top-level folder.

Not sure I agree 100%, because with the test we do not test only the bindings but also the ripser_parallel behavior. You are right that for collapser we only validate the bindings themself !

To be honest, if we can only have Python test, it is much easier to implement them. If we find something really specific to the C++ implementation that can only be tested with C++ test, then yes we should go there. But otherwise, I prefer that we push first for Python test, and only in last resort we implement C++ test.

I just think that if we bury test folder inside the python folder. I won't be easy to spot for any new user/dev that we have implemented test. Meaning that maybe it is not now the right time to move the folder. IMO this can be done later, even better when we add support to a new filtration.

[MonkeyBreaker]

Apart for the test folder, the rest of the PR LGTM.
We can merge to not keep this open IMO :)

[ulupo]

@MonkeyBreaker I see your point about visibility of the test folder! Let's keep this in mind for the future, and maybe revert back at some point.