adding rarefy #3

antgonza · 2017-05-22T16:20:42Z

This adds the first command of Q2, rarefy (note that we are using the direct biom call), and all the setup for tests and install to run fine.

coveralls · 2017-05-22T17:16:02Z

Changes Unknown when pulling bb62be7 on antgonza:rarefy into ** on qiita-spots:master**.

antgonza · 2017-05-22T17:16:41Z

Ready for review!

wasade · 2017-05-22T21:21:20Z

qp_qiime2/__init__.py

+
+# Initialize the plugin
+plugin = QiitaPlugin(
+    'qiime2', '4.2017', 'QIIME 2')


why change the version string? will this be compatible with 2017.4.x? also, note, 2017.5 will be coming out this week

No idea. Gonna swap values. I guess we need to update the string based on when the final plugin is merged and released in main qiita ...

Is it possible to import the version from qiime2 directly, so we don't have to manually update it? This way it will work with whatever version of q2 is installed.

good idea ...

wasade · 2017-05-22T21:21:48Z

qp_qiime2/__init__.py

+# Define the rarefy command
+req_params = {'i-table': ('artifact', ['BIOM'])}
+opt_params = {
+    'p-sampling-depth': ['integer', '10000']


should this be 1000 given the dflt_param_set?

That works.

wasade · 2017-05-22T21:23:11Z

qp_qiime2/qiime2.py

+    if not exists(out_dir):
+        mkdir(out_dir)
+
+    rarefied = b.subsample(rarefy_level)


should the function return False if the resulting table is empty?

Good question, gonna add a test ...

wasade · 2017-05-22T21:24:13Z

setup.py

+
+from setuptools import setup
+
+__version__ = "1.0.2"


Not sure, I think this is the default value we are using on all the plugins, @josenavas, what do you think?

I don't think that is a default value for all plugins (in fact, I think it is only used in the deblur plugin because it is matching the deblur version that it implements). Probably using the versioning from Q2 would be better.

wasade · 2017-05-22T21:24:20Z

setup.py

+    Topic :: Software Development :: Libraries :: Application Frameworks
+    Topic :: Software Development :: Libraries :: Python Modules
+    Programming Language :: Python
+    Programming Language :: Python :: 2.7


changing ...

antgonza

Thanks @wasade, addressed your comments. @josenavas, could you review?

antgonza · 2017-05-22T22:12:36Z

qp_qiime2/__init__.py

+
+# Initialize the plugin
+plugin = QiitaPlugin(
+    'qiime2', '4.2017', 'QIIME 2')


No idea. Gonna swap values. I guess we need to update the string based on when the final plugin is merged and released in main qiita ...

antgonza · 2017-05-22T22:12:47Z

qp_qiime2/__init__.py

+# Define the rarefy command
+req_params = {'i-table': ('artifact', ['BIOM'])}
+opt_params = {
+    'p-sampling-depth': ['integer', '10000']


That works.

antgonza · 2017-05-22T22:13:18Z

qp_qiime2/qiime2.py

+    if not exists(out_dir):
+        mkdir(out_dir)
+
+    rarefied = b.subsample(rarefy_level)


Good question, gonna add a test ...

antgonza · 2017-05-22T22:20:07Z

setup.py

+    Topic :: Software Development :: Libraries :: Application Frameworks
+    Topic :: Software Development :: Libraries :: Python Modules
+    Programming Language :: Python
+    Programming Language :: Python :: 2.7


changing ...

coveralls · 2017-05-22T22:30:01Z

Changes Unknown when pulling ca97949 on antgonza:rarefy into ** on qiita-spots:master**.

josenavas

Few comments, in general looks good. My question is why is this added to the qiime2 plugin if this is not using qiime2 at all?

josenavas · 2017-05-22T22:50:28Z

.travis.yml

+  - conda install --yes -n qiime2 -c anaconda flake8
+  - pip install coveralls
+  - pip install https://github.com/qiita-spots/qiita_client/archive/master.zip
+  - pip install https://github.com/qiita-spots/qiita-files/archive/master.zip


I don't think this is a dependency, remove.

josenavas · 2017-05-22T22:51:39Z

qp_qiime2/__init__.py

+
+# Initialize the plugin
+plugin = QiitaPlugin(
+    'qiime2', '4.2017', 'QIIME 2')


Is it possible to import the version from qiime2 directly, so we don't have to manually update it? This way it will work with whatever version of q2 is installed.

josenavas · 2017-05-22T22:52:25Z

qp_qiime2/__init__.py

+
+# Define the rarefy command
+req_params = {'i-table': ('artifact', ['BIOM'])}
+opt_params = {


I would put p-sampling-depth as a required parameter and not provide a default value - I think users should decide this value since it is really study dependent. Having a default value may mislead users.

josenavas · 2017-05-22T22:53:11Z

qp_qiime2/__init__.py

+}
+qiime_cmd = QiitaCommand(
+    "Rarefy", "Rarefy",
+    rarefy, req_params, opt_params, outputs, dflt_param_set)


I would add analysis_only=True since we want this to be available only in the analysis pipeline.

tried but I think this in the latest version of the qiita_client:

Traceback (most recent call last): File "qp_qiime2/tests/test_qiime2.py", line 21, in <module> from qp_qiime2 import plugin File "/Users/antoniog/svn_programs/qp-qiime2/qp_qiime2/__init__.py", line 35, in <module> analysis_only=True) TypeError: __init__() got an unexpected keyword argument 'analysis_only'

oh - I see. I though the PR was merged. Just checked and there was a test failure on py3 (everything passes on py2). Give me a few minutes and I will update that PR.

josenavas · 2017-05-22T22:57:18Z

setup.py

+      scripts=['scripts/configure_qiime2', 'scripts/start_qiime2'],
+      extras_require={'test': ["nose >= 0.10.1", "pep8"]},
+      install_requires=['click >= 3.3', 'future'],
+      dependency_links=[


Remove since qiita-files is not a dependency.

josenavas · 2017-05-22T22:59:10Z

setup.py

+
+from setuptools import setup
+
+__version__ = "1.0.2"


I don't think that is a default value for all plugins (in fact, I think it is only used in the deblur plugin because it is matching the deblur version that it implements). Probably using the versioning from Q2 would be better.

antgonza

thanks @josenavas

antgonza · 2017-05-23T02:59:29Z

.travis.yml

+  - conda install --yes -n qiime2 -c anaconda flake8
+  - pip install coveralls
+  - pip install https://github.com/qiita-spots/qiita_client/archive/master.zip
+  - pip install https://github.com/qiita-spots/qiita-files/archive/master.zip


antgonza · 2017-05-23T03:04:28Z

qp_qiime2/__init__.py

+
+# Initialize the plugin
+plugin = QiitaPlugin(
+    'qiime2', '4.2017', 'QIIME 2')


good idea ...

antgonza · 2017-05-23T03:05:24Z

qp_qiime2/__init__.py

+
+# Define the rarefy command
+req_params = {'i-table': ('artifact', ['BIOM'])}
+opt_params = {


antgonza · 2017-05-23T03:05:47Z

qp_qiime2/__init__.py

+}
+qiime_cmd = QiitaCommand(
+    "Rarefy", "Rarefy",
+    rarefy, req_params, opt_params, outputs, dflt_param_set)


antgonza · 2017-05-23T03:06:05Z

setup.py

+
+from setuptools import setup
+
+__version__ = "1.0.2"


antgonza · 2017-05-23T03:09:52Z

qp_qiime2/__init__.py

+}
+qiime_cmd = QiitaCommand(
+    "Rarefy", "Rarefy",
+    rarefy, req_params, opt_params, outputs, dflt_param_set)


tried but I think this in the latest version of the qiita_client:

Traceback (most recent call last): File "qp_qiime2/tests/test_qiime2.py", line 21, in <module> from qp_qiime2 import plugin File "/Users/antoniog/svn_programs/qp-qiime2/qp_qiime2/__init__.py", line 35, in <module> analysis_only=True) TypeError: __init__() got an unexpected keyword argument 'analysis_only'

coveralls · 2017-05-23T03:20:39Z

Changes Unknown when pulling 0e80429 on antgonza:rarefy into ** on qiita-spots:master**.

josenavas · 2017-05-23T18:21:44Z

setup.py

+
+from setuptools import setup
+
+from qiime2 import __version__ as qiime2_version


my only concern about having this like it is right now is that this package assumes that q2 is already installed in order to install this package. Are we ok given that q2 is not pip installable and the recommended installation procedure is conda? If so, can you add a note on the README stating that this package assumes that Q2 is already installed on the active environment, and add a link to q2's installation instructions?

I can do that just note that we are doing this based on your suggestion ... pr in a second ...

josenavas · 2017-05-23T18:30:54Z

README.rst

@@ -0,0 +1,4 @@
+# qp-qiime2


Note that by changing the file format from markdown (md) to rst, the formatting of the file has been lost (check on your branch). Can you either leave it as an md file or format it correctly?

Thanks

coveralls · 2017-05-23T18:33:08Z

Changes Unknown when pulling 2aafdea on antgonza:rarefy into ** on qiita-spots:master**.

antgonza · 2017-05-23T20:19:07Z

k, thanks. change made.

coveralls · 2017-05-23T20:26:15Z

Changes Unknown when pulling 83dd7ed on antgonza:rarefy into ** on qiita-spots:master**.

coveralls · 2017-05-23T20:32:22Z

Changes Unknown when pulling 83dd7ed on antgonza:rarefy into ** on qiita-spots:master**.

wasade · 2017-06-07T18:55:52Z

I'm 👍

antgonza added 5 commits May 22, 2017 10:19

adding rarefy

d94aed2

rm --file

e77c1ec

resorting script section

662fe9c

install flake8 qiime2

0791a66

conda install --yes

bb62be7

wasade reviewed May 22, 2017

View reviewed changes

addressing @wasade comments

ca97949

antgonza commented May 22, 2017

View reviewed changes

josenavas suggested changes May 22, 2017

View reviewed changes

addressing @josenavas comments

0e80429

antgonza commented May 23, 2017

View reviewed changes

josenavas reviewed May 23, 2017

View reviewed changes

qiime2 install @josenavas

2aafdea

josenavas reviewed May 23, 2017

View reviewed changes

README.rst -> README.md

a322e7b

analysis_only=True

83dd7ed

josenavas merged commit fa67440 into qiita-spots:master Jun 7, 2017


		from setuptools import setup

		from qiime2 import __version__ as qiime2_version

adding rarefy #3

adding rarefy #3

Conversation

antgonza commented May 22, 2017

coveralls commented May 22, 2017

antgonza commented May 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antgonza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 22, 2017

josenavas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antgonza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 23, 2017

antgonza commented May 23, 2017

coveralls commented May 23, 2017

coveralls commented May 23, 2017

wasade commented Jun 7, 2017