Hi! incredible promises in your Readme.md (speed through Cython, easy API, etc.)
But I found the library a bit unfinished, so here are some (hopefully) constructive feedback:
- Import error. After installing via
git clone etc. in clean env, as required by readme.md, and just doing import gmatch4py as gm, I still encounter an import issue:
from .embedding.deepwalk import *
File "gmatch4py/embedding/deepwalk.pyx", line 29, in init gmatch4py.embedding.deepwalk
ModuleNotFoundError: No module named 'graph'
-
Autocompletion not working: maybe it's just in my IDE (Pycharm Pro), but I have no autocomplete of functions signatures, or no doctsrings displayed°. Do you have on your side?
2 bis) I know it takes time, but docs are not ultra clear for some functions (for example, how does .set_attr_graph_used works, how does it actually "use" attributes)
-
aka 2 bis²) : why does .set_attr_graph_used always took 2 arguments, can't we just use edge attr or node attr ? It seems not, but it's not clear to me why.
TypeError: set_attr_graph_used() takes exactly 2 positional arguments (1 given)
° I think it might be because of compiled files .so. From my researchs, it seems docstrings are dropped by defaullt. But here, they give an option to keep docstrings:
Cython.Compiler.Options.docstrings = True
Have a nice day, I hope my feedback will be useful. Again, thanks for open-sourcing your work!
Nicolas MICAUX
Hi! incredible promises in your Readme.md (speed through Cython, easy API, etc.)
But I found the library a bit unfinished, so here are some (hopefully) constructive feedback:
git cloneetc. in clean env, as required by readme.md, and just doingimport gmatch4py as gm, I still encounter an import issue:Autocompletion not working: maybe it's just in my IDE (Pycharm Pro), but I have no autocomplete of functions signatures, or no doctsrings displayed°. Do you have on your side?
2 bis) I know it takes time, but docs are not ultra clear for some functions (for example, how does
.set_attr_graph_usedworks, how does it actually "use" attributes)aka 2 bis²) : why does
.set_attr_graph_usedalways took 2 arguments, can't we just use edge attr or node attr ? It seems not, but it's not clear to me why.TypeError: set_attr_graph_used() takes exactly 2 positional arguments (1 given)° I think it might be because of compiled files
.so. From my researchs, it seems docstrings are dropped by defaullt. But here, they give an option to keep docstrings:Cython.Compiler.Options.docstrings = TrueHave a nice day, I hope my feedback will be useful. Again, thanks for open-sourcing your work!
Nicolas MICAUX