Closed kinow closed 4 years ago
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 (recipe
) and found it was in an excellent condition.
I do have some suggestions for making it better though...
For recipe:
Documentation on acceptable licenses can be found here.
@conda-forge-admin, please rerender
Oh, just thinking, we've added some more optional dependencies in 8.0a2, how do handle those?
https://github.com/cylc/cylc-flow/blob/master/setup.py#L69-L88
Oh, just thinking, we've added some more optional dependencies in 8.0a2, how do handle those?
https://github.com/cylc/cylc-flow/blob/master/setup.py#L69-L88
Oh! Good question! I never thought about the optional dependencies in extras_requires
. Let me see what's the equivalent. We will need to either use whatever Conda offers, or maybe add all the possible dependencies here I think.
I guess you can now complete the checklist in the description?
I left the bumped up option unchecked to show that that did not change. The item below it is also about the build number. Maybe I should remove the one about bumping it up? (their template doesn't say so)
Conda issue about optional dependencies: https://github.com/conda/conda/issues/1696
There is no direct equivalent for extras_require
in Conda: https://github.com/conda/conda/issues/3299
From what I understood, the recommended way in Conda, is to have metapackages: https://docs.conda.io/projects/conda-build/en/latest/resources/commands/conda-metapackage.html. Or, to create multiple outputs from the same recipe.
JupyterHub does the latter: https://github.com/conda-forge/jupyterhub-feedstock/blob/c9c9185c37ed8b06f2b52e9df704f32e99eeff9c/recipe/meta.yaml#L22
~I think we are good for now, as our only extra dependencies are for tests now. But I think we will need to create an issue in this repository to sort out this. WDYT @oliver-sanders / @hjoliver ?~
I had missed the entries in setup.py
, but thanks to @oliver-sanders ' link I spotted my mistake. So... should we just include EmPy
, pandas
, pympler
, and matplotlib
to our dependencies in this recipe for now?
There is an open conda issue for optional dependencies of the pip kind (the older one mentioned seems to have been closed by different "constraints for optional dependencies"): https://github.com/conda/conda/issues/7502
So... should we just include EmPy, pandas, pympler, and matplotlib to our dependencies in this recipe for now?
My feeling is yes. Otherwise (pending conda/conda#7502) we have to make a separate Cylc metapackage for each "option" - is that right?
So... should we just include EmPy, pandas, pympler, and matplotlib to our dependencies in this recipe for now?
My feeling is yes. Otherwise (pending conda/conda#7502) we have to make a separate Cylc metapackage for each "option" - is that right?
I think that's correct, and that may change before 8.0, we may have more or less groups. Will add the other dependencies just in case, then Oliver and others can review it later :+1:
There is an open conda issue for optional dependencies of the pip kind (the older one mentioned seems to have been closed by different "constraints for optional dependencies"): conda/conda#7502
There's a comment in this issue about the outputs, which is used in JupyterHub too. We would end up with something like
cylc-flow
(only install_requires
)cylc-flow.empy
(or -empy
, etc)cylc-flow.reporting
Though there's no convention yet in Conda, as in pip (i.e. pip install cylc-flow[empy]
meaning that the []
delimiter a group of dependencies, where cylc-flow.empy
doesn't specify that that's necessarily cylc-flow + empy).
Looks like that may change a bit. I'm watching that issue now for notifications. Thanks @hjoliver !
Done, see latest commit.
Thanks @hjoliver!
Approving, but I'll request re-review from @oliver-sanders to make sure he agrees with including all the optional deps at this stage.
I guess, we can refine this later.
I think I remember reading about conda "outputs" at some point but can't find anything on the interweb at the moment.
[edit] just saw your comment above, I like the jupyterhub approach, should try it out for 8.0a3 (which will hopefully be ready pretty soon).
Checklist
0
(if the version changed)conda-smithy
(Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)