bokulich-lab / q2-fondue

Functions for reproducibly Obtaining and Normalizing Data re-Used from Elsewhere
BSD 3-Clause "New" or "Revised" License
20 stars 6 forks source link

FIX: readme update #46

Closed adamovanja closed 3 years ago

adamovanja commented 3 years ago

minor edits to the ReadMe file (aligned with creation of tutorial) including testing plugin with Q2 2021.11tested version.

LenaFloerl commented 3 years ago

Hi - this looks good to me! 🌻

Only one remark: I would suggest changing the installation instructions to specify that q2-fondue should be installed in an existing QIIME 2 environment. Maybe similar to what I wrote in the tutorial draft:

q2-fondue will be installable as a conda package in the near future. For now, please install it in an existing QIIME 2 environment.

First activate your QIIME 2 environment and install relevant dependencies:

conda activate qiime2-2021.8

conda install -c conda-forge -c bioconda -c defaults \
  qiime2 q2cli q2-types "entrezpy>=2.1.2" \
  "sra-tools==2.9.6" xmltodict "tzlocal==2.1"

Next, install q2-fondue:

pip install git+https://github.com/bokulich-lab/q2-fondue.git

Finally, update your QIIME 2 libraries:

qiime dev refresh-cache

Followed by the developer note you updated.

adamovanja commented 3 years ago

hi @LenaFloerl, thank you for your review. Actually the current ReadMe commands are already installing q2-fondue into an existing environment. Here we just don't assume that the user already has a Q2 conda environment set up and show how to set it up with the requirements. I have edited the text around it to make this a bit clearer. Let me know if this is what you meant.

misialq commented 3 years ago

I would only add one thing: I'm wondering whether it makes sense to update the dev note for the following reasons:

I'd rather say we could drop it entirely (see also https://github.com/bokulich-lab/q2-fondue/pull/34#discussion_r744460018)

adamovanja commented 3 years ago

I agree @misialq. I adjusted it accordingly. If you both agree I will merge this PR then.