bokulich-lab / RESCRIPt

REference Sequence annotation and CuRatIon Pipeline
BSD 3-Clause "New" or "Revised" License
84 stars 26 forks source link

Update of README. #170

Closed mikerobeson closed 5 months ago

mikerobeson commented 5 months ago

It's been a while since we've updated the README. My proposed changes are in this PR. This was motivated by this forum thread.

lizgehret commented 5 months ago

Thanks @mikerobeson ! Indeed, this readme needs a good dusting off 😁

I have a few minor comments in-line, but overall my primary thoughts are about the install instructions. It would be nice to simplify the install instructions, as installation is not as complicated now as it was before.

RESCRIPt is already included in the shotgun distro, so a very easy installation instruction could just be to install that distro. We could drop the alternative options and just redirect to the install instructions for that distro.

I think RESCRIPt is also going to be added to the amplicon distro in the next release — is that correct, @lizgehret? Or in a future release.

Sorry for the delayed response @nbokulich! Yes, we're aiming to get RESCRIPt in the amplicon distro in this upcoming release - which I think should be doable because @hagenjp will be working on moving all of the types/formats/etc from q2-types-genomics over to q2-types.

mikerobeson commented 5 months ago

I just put up another draft of the README. Still a work in progress, but wanted to put forth some ideas on how to clarify installation, etc. :-)

nbokulich commented 5 months ago

Hey @mikerobeson I think that the number of install options is getting a bit overwhelming. I think that options 1 and 2 are enough. Maybe move the other options to a side document? And we could just drop a link to that in the README. This would mostly be for posterity and probably not useful to anyone after another release or two.

mikerobeson commented 5 months ago

number of install options is getting a bit overwhelming

I agree @nbokulich. I really like your idea of a side document. I can just link to that and say something like, "For additional notes on installing other versions of RESCRIPt please see the following document".

mikerobeson commented 5 months ago

Just pushed a new draft update of the README.md, and added the supplementary file install-prior-versions.md.

mikerobeson commented 5 months ago

Should we remove the lint-build-test badge? Or is there a way to fix it or point to something else?

nbokulich commented 5 months ago

Hey @mikerobeson thanks for the updates! LGTM. Are we ready to merge or do we wait for 2024.2 to release?

Yeah thanks for spotting the broken badge, the badge should have been changed when the CI was updated. I will investigate this and open a separate PR for the sake of simplicity/clarity, instead of giving you more homework on this PR! 😂

mikerobeson commented 5 months ago

I think this is good to go for merging.

mikerobeson commented 5 months ago

I'll accept and merge your badge PR and then update this PR.

mikerobeson commented 5 months ago

Okay @nbokulich. This should be good to merge once the checks pass. :-)

nbokulich commented 5 months ago

Hey @mikerobeson sorry, still not ready to merge, I caught something else.

At first I was going to suggest that we wait to merge until 2024.2 is released later this month, as some of the links that you give in the instructions (e.g., https://docs.qiime2.org/2024.2/install/native/) are not yet active and only will be after the release.

But then I realized that those instructions do not make sense anyway, as RESCRIPt will be part of the 2024.2 amplicon distro.

So I think that the installation instructions should instead have the following options/structure:

  1. "Install as part of a QIIME 2 distribution": Refer to the QIIME 2 documentation for the latest installation instructions and install the q2-shotgun distribution. (then once 2024.2 is released we can add the amplicon distro to this list). I would just point to docs.qiime2.org so that the link does not become stale if pointing to a release-specific install doc.
  2. Minimal RESCRIPt environment (fine as is, but let's swap the order)
  3. Prior versions of RESCRIPt. (fine as is, but maybe let's say "Prior versions of RESCRIPt / QIIME 2"? As these would be the instructions if trying to install in a pre-2024.2 env)

Does that sound okay? Sorry for the 11th-hour changes!

mikerobeson commented 5 months ago

That all makes sense to me @nbokulich! 👍

mikerobeson commented 5 months ago

Hey @nbokulich, I realized that I need to update the commands users to install the "Minimal RESCRIPt environment". I am unable to get this to work. This is the closest to success I've been able to get:

conda install \
  -c https://packages.qiime2.org/qiime2/2023.9/shotgun/passed/ \
  -c qiime2 -c conda-forge -c bioconda -c defaults \
  qiime2 q2cli q2templates q2-types q2-types-genomics q2-longitudinal q2-feature-classifier \
  "pandas>=0.25.3" xmltodict ncbi-datasets-pylib

I've tried a variety of things, like switching the channel order, altering allowable versions, etc. But I keep getting this error (when using mamba to install):

q2-types-genomics is not installable because it requires └─ qiime2 2023.9.* , which conflicts with any installable versions previously reported.

I'm not sure of the best way to go about resolving this. Any ideas? 🤷‍♂️

nbokulich commented 5 months ago

maybe we just drop the minimal install instructions? We could figure out how to add RESCRIPt to the "tiny" distro, but otherwise rescript depends on a few plugins so installing as part of a distro is simplest and probably best.

mikerobeson commented 5 months ago

I think I figured it out! If I simply add both shotgun and amplicon 'passed' channels it works:

conda create -y -n rescript
conda activate rescript
conda install \
  -c https://packages.qiime2.org/qiime2/2023.9/shotgun/passed/ \
  -c https://packages.qiime2.org/qiime2/2023.9/amplicon/passed/ \
  -c conda-forge -c bioconda -c qiime2 -c defaults \
  qiime2 q2cli q2templates q2-types q2-types-genomics q2-longitudinal q2-feature-classifier \
  "pandas>=0.25.3" xmltodict ncbi-datasets-pylib

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

Perhaps a middle ground? That is, I can just append these "minimal environment" instructions to the install-prior-versions.md document? 🤔

mikerobeson commented 5 months ago

I updated another draft, leaving the "minimal install" option in the README. But if you really think we should drop it, I'll move it to the supplementary instructions. I just feel we should provide a minimal option somewhere.

mikerobeson commented 5 months ago

@nbokulich, I moved the minimal install to the supplement. I also removed the pip command and updated the conda install to include rescript. It all seems to work.