search

netsiphd / netrd

A library for network {reconstruction, distances, dynamics}
https://netrd.readthedocs.io/en/latest/
MIT License
166 stars 43 forks source link

Replace earthmover's distance implementation #280

Closed sdmccabe closed 4 years ago

sdmccabe commented 4 years ago

This resolves #232 and helps with #267 by replacing the POT-based earthmover's distance implementation with Jeremy Kun's newly MIT-licensed implementation based on ortools. His implementation is not on pypi so the relevant functions are reproduced (with the license) in nbd.py. This removes the Cython and POT workarounds we had in .travis.yml and requirements.txt to handle its installation. This will hopefully make for a smoother experience with test builds.

sdmccabe commented 4 years ago

image

sdmccabe commented 4 years ago

Is including the license at the bottom of nbd.py the way I've done sufficient?

sdmccabe commented 4 years ago

Seems confusing/troubling that the build passed before I had added ortools to requirements.txt; should double-check that before merging.

sdmccabe commented 4 years ago

And now it's failing with ortools in requirements.

sdmccabe commented 4 years ago

The tests passed in the previous case because NBD wasn't imported in the case of an ImportError in that module, a workaround we had previously used when adding NBD. So this all looks good to me, as long as it (in particular the license stuff) looks good to other people.