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

add bioservices #14302

Merged
merged 4 commits into from
Apr 14, 2021
Merged

add bioservices #14302

merged 4 commits into from
Apr 14, 2021

Conversation

hadim
Copy link
Member

@hadim hadim commented Mar 19, 2021

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/bioservices) and found some lint.

Here's what I've got...

For recipes/bioservices:

  • noarch: python recipes are required to have a lower bound on the python version. Typically this means putting python >=3.6 in both host and run but you should check upstream for the package's Python compatibility.

For recipes/bioservices:

  • Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.
  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/bioservices) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/bioservices:

  • Recipe with the same name exists in bioconda: please discuss with @conda-forge/bioconda-recipes.

@hadim
Copy link
Member Author

hadim commented Mar 19, 2021

Ping @conda-forge/bioconda-recipes about this package. Are you ok adding it to conda-forge?

@hadim
Copy link
Member Author

hadim commented Mar 19, 2021

@conda-forge/staged-recipes please review

@jakirkham
Copy link
Member

LGTM. Had one question above

@jakirkham
Copy link
Member

Just noticed they also mention BSD-3-Clause despite including GPL 3. Asking about that here ( cokelaer/bioservices#187 )

@hadim
Copy link
Member Author

hadim commented Mar 19, 2021

Just noticed they also mention BSD-3-Clause despite including GPL 3. Asking about that here ( cokelaer/bioservices#187 )

Thanks for doing that.

@hadim
Copy link
Member Author

hadim commented Mar 22, 2021

Is that ok to merge here and fix the license once that has been clarified? Or is it a blocker in your opinion @jakirkham ?

@jakirkham
Copy link
Member

I've tried to poke that issue. It feels a little messy atm since there is a wide gap between GPL-3 and BSD-3. I think just getting an answer from the author would help

@jakirkham
Copy link
Member

cc @conda-forge/bioconda-recipes (as it looks like the same recipe may be in Bioconda)

@hadim
Copy link
Member Author

hadim commented Apr 9, 2021

The licensing seems to have been clarified. Is that ok to merge @jakirkham or do we need specific approval from bioconda folks?

@jakirkham
Copy link
Member

We need feedback from Bioconda since they already package this

@mbargull
Copy link
Member

mbargull commented Apr 9, 2021

Hi @hadim and @jakirkham, sorry for the late reply.
The license should be GPL-3.0-only since the source does not seem to have the "or any later version" phrase in it (apart from the GPL text itself which just describes on how to apply it).
What is the intend on moving this over from Bioconda?

Cheers,
Marcel
(I'm going AFK for today, but will get back to you in the following days.)

@hadim
Copy link
Member Author

hadim commented Apr 9, 2021

@mbargull the main motivation for moving it on conda-forge is to rely only on the conda-forge channel to build our stack. As adding more channels tends to the complexity the graph of deps solving step with conda/mamba.

@bgruening
Copy link
Contributor

@hadim thats ok. Just make sure you remove the recipe from the bioconda side.

@hadim
Copy link
Member Author

hadim commented Apr 13, 2021

Is that ok to merge here?

@bgruening once merged here I can remove the recipe from bioconda. Is there is a specific procedure to follow?

@mbargull mbargull merged commit fa35a31 into conda-forge:master Apr 14, 2021
@mbargull
Copy link
Member

Sorry for the delay.

As adding more channels tends to the complexity the graph of deps solving step with conda/mamba.

Understandable, both implementations have different deficiencies with this, indeed.

@bgruening once merged here I can remove the recipe from bioconda. Is there is a specific procedure to follow?

Yes, please open a PR on bioconda-recipes to remove the recipe with a reference to this PR; much appreciated!

@hadim
Copy link
Member Author

hadim commented Apr 14, 2021

Thank you. See the PR bioconda/bioconda-recipes#28010

@cokelaer
Copy link
Contributor

cokelaer commented Sep 9, 2021

@hadim @bgruening
I'm the main author of bioservices and I am not sure I understood the rational in moving bioservices from bioconda to conda-forge. BioServices is indeed a software that is dedicated to bioinformatics and biological web services access and I have the feeling it would be more visible in bioconda rather than conda-forge ? For end-users, it should be equivalent of course but I ust want to make sure that it makes sense before updating the documentation. thanks

@hadim
Copy link
Member Author

hadim commented Sep 9, 2021

@cokelaer I think the main motivation is to reduce the number of conda channels in a custom conda environment. Having a lower number of channels reduces the dep graph solver computational time and in consequence reduces the conda env creation in CI, local machines, docker images, production, etc.

Since (if I am not mistaken), bioconda is relying on conda-forge as you said it should not change anything to the main user.

As for the visibility I understand having it in conda-forge makes it less visible but maybe this is not the most important channel of communication?

I apologize if you feel strongly against the move.

@cokelaer
Copy link
Contributor

@hadim thanks for the quick reply and explanations. If it does not change anything for the user, I believe this move is for the best. thanks for your work. best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants