biocore / mds-approximations

Multidimensional scaling algorithms for microbiology-ecology datasets.
6 stars 7 forks source link

PCoA updates #46

Closed HannesHolste closed 5 years ago

HannesHolste commented 6 years ago

This PRs brings this repo in sync with changes made for https://github.com/biocore/scikit-bio/pull/1569

  1. Don't normalize eigenvectors to unit length (doesn't change correctness of results anyway, just rescaling)
  2. Compute proportions explained via eigenvalues obtained by matrix trace. For details see notes on code changes in https://github.com/biocore/scikit-bio/pull/1569
antgonza commented 6 years ago

Just to be sure, this implies that the same code is in skbio and this repo, right? If that's the case, will it be better to erase the code from here and use skbio's?

HannesHolste commented 6 years ago

@antgonza: Correct. Good suggestion, and I think it's best to do so at a later date, after the FSVD is merged into fsvd. I'd just like to get this repo up to speed with Rob's requirements to have all work public for now.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.4%) to 87.779% when pulling bddacc1d9d25400d3dff5fa6a9661240ff5aa2b8 on pcoa_updates into 5911f9ca5c02c342ec4829f4d8411b876b9b781c on master.

antgonza commented 6 years ago

My suggestion is to focus in addressing all the issues in https://github.com/biocore/scikit-bio/pull/1569 so it can be merge and then continue with this.

HannesHolste commented 5 years ago

FSVD is now part of skbio. Perhaps just remove 'raw' fsvd code from this repo completely, and replace with a call to skbio's fsvd?

antgonza commented 5 years ago

Replacing sounds great!! Thanks

HannesHolste commented 5 years ago

@antgonza fixed travis issues and completed this PR. Had to upgrade miniconda and python (to 3.x), because skbio requires py3 now.

Edit: I tested this locally with a new conda environment and all tests pass. But travis CI on here keeps failing, and I don't see obvious reasons why. Do you have any suggestions? I'm a bit stuck with debugging this.

antgonza commented 5 years ago

@HannesHolste, there are still build errors ... do you want me to try to fix?

HannesHolste commented 5 years ago

@antgonza yes I need your input on this, I'm stuck on why this isn't working on travis. I don't see anything blatantly wrong with the config. Tried clean conda env and all tests pass locally...

antgonza commented 5 years ago

OK, will take a look later today, thanks!

antgonza commented 5 years ago

Finally was able to get back to this, sorry for the delay. Anyway, a couple of notes: