search

giotto-ai / giotto-ph

High performance implementation of Vietoris-Rips persistence.
https://giotto-ai.github.io/giotto-ph/
Other
43 stars 12 forks source link

Allow users to import ripser_parallel more intuitively #15

Closed ulupo closed 2 years ago

ulupo commented 3 years ago

At the moment, Python users have to do from gph.python import ripser_parallel. It is maybe a bit weird to have to import a subpackage called "python" in a python program. Should we change this, moving forward, to one of:

  1. from gph import ripser_parallel (super quick)?
  2. from gph.ripser import ripser_parallel (similar to ripser.py, more future proof when giotto-ph contains other filtrations)?
MonkeyBreaker commented 3 years ago

I am all to remove the python part in import if it is possible. I have a personal preference for 1., but I can have a look on best practice this afternoon.

MonkeyBreaker commented 3 years ago

I have something that seems to work with proposal 1. I was not able to have something functional for 2. but if you know how to make it work, please let me know because I am interested !

Let me know if you want me to prepare a PR with 1.

ulupo commented 3 years ago

Thanks for the reply @MonkeyBreaker !

In principle, one might be able to combine both approaches. I just think that, in terms of subpackage structure, ripser should probably already be made as a subpackage now if we have any future ambitions to extend to other filtrations. This is why I mentioned approach 2, but with the right __init__ files I think we can achieve both approaches.

MonkeyBreaker commented 3 years ago

Quick question for suggestion 2, should we call ripser or vr ?

ulupo commented 3 years ago

Quick question for suggestion 2, should we call ripser or vr ?

I think your suggestion of vr is better!