Bumping to Latest Topopy version #5
Conversation
maljovec
commented
Feb 27, 2020
maljovec
commented
- Setting bounds on topopy version
- Explicitly requiring nglpy
* Explicitly requiring nglpy
| topo = MorseSmaleComplex(graph=graph, gradient=gradient, max_neighbors=knn, beta=beta, | ||
| normalization=norm, aggregator=aggregator, connect=connect) | ||
| # topopy ver 1.0: remove names | ||
| # topo.build(X=data.x.values, Y=y.values, names=list(data.x.columns)+[y.name]) |
| topo.build(X=data.x.values, Y=y.values) | ||
|
|
||
| # topopy version 1.0.0 | ||
| # graph = ngl.EmptyRegionGraph(beta=beta, relaxed=False, p=2.0) | ||
| # topo = MorseSmaleComplex(graph=graph, gradient=gradient, normalization=norm, aggregator=aggregator) |
There was a problem hiding this comment.
More or less copied to above.
| @@ -1,4 +1,5 @@ | |||
| numpy | |||
| sklearn | |||
| topopy | |||
| nglpy>=1.0.6,<2.0 | |||
| topopy>=1.0,<2.0 | |||
There was a problem hiding this comment.
I am explicitly pinning versions here, to coincide with semantic versioning. If I bump the major topopy version again, it would be a breaking API change, otherwise, minor version and patch version changes should be grabbed.
| @@ -39,7 +39,7 @@ def get_version(file, name='__version__'): | |||
| zip_safe=False, | |||
| packages=find_packages(exclude=('tests', 'docs')), | |||
| install_requires=[ | |||
| 'topopy>=0.1', 'numpy', 'sklearn', 'pandas' | |||
| 'nglpy>=1.0.6,<2.0', 'topopy>=1.0,<2.0', 'numpy', 'sklearn', 'pandas' | |||
There was a problem hiding this comment.
It is up to you, but it may be easier to load all of the requirements.txt and fill this section in here, so you don't have to constantly update twice. They do have different meanings, I suppose, but for all intents and purposes I think a python setup.py or a pip should grab the prerequisites for you. 🤷♂
| # topopy ver 1.0: remove names | ||
| # topo.build(X=data.x.values, Y=y.values, names=list(data.x.columns)+[y.name]) | ||
| if 'knn' in graph: | ||
| beta=0.0 |
There was a problem hiding this comment.
I should test this, but my thinking is this will essentially make the prune operation of NGL a no-op, this will be an inefficient no-op, but alleviates us from having to write a custom graph. Maybe I can put the logic in nglpy to short-circuit if 0 is given for this parameter. If I were smarter and used the same API as sklearn, then you could just pass a sklearn.kneighbors instead and it would just work. This should just be a matter of renaming build to fit for nglpy...
