Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify delivery and interfacing #111

Open
mstrazar opened this issue Dec 10, 2019 · 5 comments
Open

Simplify delivery and interfacing #111

mstrazar opened this issue Dec 10, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@mstrazar
Copy link
Contributor

Hi guys,
congratulations for the Nature methods paper on this.

I have a couple of suggestions:

  • support running the tool from more standard input files rather than .biom (.csv, .tab. etc.), that do not need special libraries to handle.
  • specify versions of all python libraries needed. Currently, this does not install, as you appear to be using an old version of tensorflow. Even better, provide a Docker image.
  • minimize dependencies as far as possible. Even better, provide a Docker image.

When other labs will try to publish similar methods, they will be expected to compare the results to your method and these will make peoples life easier.

Kind regards,
Martin

@mortonjt mortonjt added the bug Something isn't working label Dec 10, 2019
@mortonjt
Copy link
Collaborator

Hi @mstrazar , thank you for interest in mmvec! PR raised about setup.py update: #112

Regarding installation, what have you tried so far? The conda installation has TF pinned.
Could you post any errors?

Regarding Docker containers - we actually have them, that's what is running on the backend of GNPS.

@mwang87 are there tutorials on how to pull down these Docker containers?
It may not be bad idea to have these in the main README.

@mortonjt
Copy link
Collaborator

Regarding biom tables as input, there are no plans for passing in csv or tab, due to their inability to adequately represent sparse data. Furthermore, biom is also a standard file format, and is a core component of this library.

See this paper for more details: https://gigascience.biomedcentral.com/articles/10.1186/2047-217X-1-7

@mstrazar
Copy link
Contributor Author

Thank you for your feedback. I am keen to try mmvec on our data. Could you point me to a Docker image with mmvec installed, if it exists?

I have try installing the software with pip, but it doesn't play well with latest versions of some packages, namely tensorflow and perhaps others.

@mortonjt
Copy link
Collaborator

Hi @mstrazar , we just cut a new release of mmvec that pins Tensorflow.
Tensorflow 2.0 broke compatibility (which happened right before the paper went live) - so installing that will definitely break things.

Let us know if the newest pip install works. @mwang87 has a better idea about the Docker containers.

@mstrazar
Copy link
Contributor Author

Hi,

so I managed to pip install mmvec on a Mac OSX by creating a clean virtual environment with Python 3.6.3.

Also, one has to specifically pip install numpy first.

Previous tries:
[NO] Python 3.6.0 (Incompatibilities with the typing module)
[NO] Python 3.7.4 (Biom-format does not have wheel files)

You might want to fix versions and specify these details in the README.

Happy holidays,
Martin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants