Use multiple `outputs` for `dask` & `distributed` #172

jakirkham · 2022-02-26T00:03:49Z

Combines dask-core & distributed into a single recipe with dask by using multiple outputs to produce the different packages.

Checklist

Used a personal fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
Ensured the license file is being packaged.

conda-forge-linter · 2022-02-26T00:06:11Z

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.

conda-forge-linter · 2022-02-26T00:23:25Z

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 (recipe) and found some lint.

Here's what I've got...

For recipe:

The outputs section contained an unexpected subsection name. entry_points is not a valid subsection name.

jakirkham · 2022-02-26T00:33:02Z

The outputs section contained an unexpected subsection name. entry_points is not a valid subsection name.

Seems to be covered by conda-build's tests. So this lint doesn't seem right

Raised as issue ( conda-forge/conda-smithy#1598 )

…nda-forge-pinning 2022.02.25.15.35.46

jakirkham

Have done some work to combine dask-core & distributed into this recipe so we can update just one feedstock based on our recent conversation here ( conda-forge/dask-core-feedstock#59 (comment) ). Highlighted some differences that come out of this approach below

jakirkham · 2022-02-26T00:50:27Z

recipe/meta.yaml

+      run_constrained:
+        - cytoolz >=0.8.2
+        - numpy >=1.18
+        - pandas >=1.0
+        - distributed {{ version }}
+        - bokeh >=2.1.1
+        - jinja2


These come from the extra requirements in setup.py for dask (with exception for cytoolz, which is added to line up with toolz)

jakirkham · 2022-02-26T00:51:09Z

recipe/meta.yaml

+    build:
+      noarch: python
+      number: {{ number }}
+    script: build-dask-core.sh


We are no longer able to list commands in the recipe directly with outputs. So move the install steps to this script.

jakirkham · 2022-02-26T00:51:57Z

recipe/meta.yaml

+    entry_points:
+      - dask-scheduler = distributed.cli.dask_scheduler:go
+      - dask-ssh = distributed.cli.dask_ssh:go
+      - dask-worker = distributed.cli.dask_worker:go


As noted above the linter does not like this, but as seen from CI this builds without issue. See upstream issue for resolution of lint

recipe/meta.yaml

jakirkham · 2022-02-26T00:54:13Z

recipe/build-dask-core.sh

+#!/bin/bash
+
+pushd "${SRC_DIR}/dask"
+$PYTHON -m pip install . --no-deps --ignore-installed -vv


pip was trying to install things when it shouldn't have. So needed to add --no-deps to bypass this. pip was still trying to do some things so used --ignore-installed to clamp down on the remaining issues.

I really thought we had centrallized these issues. However, I alwyas see pip changing how they work with their flags.

I had to recently add --no-build-isolation as well. I think :/

This might be an issue related to conda/conda-build#4292

Yeah that does look like it, Jaime. Have seen this in other split recipes as well.

recipe/meta.yaml

jakirkham · 2022-02-26T00:57:27Z

recipe/meta.yaml

 about:
  home: https://dask.org/
  summary: Parallel PyData with Task Scheduling
  license: BSD-3-Clause
-  license_file: LICENSE.txt
+  license_file: dask/LICENSE.txt
  description: |
    Dask is a flexible parallel computing library for analytics.
  doc_url: https://dask.org/


We still need the metadata in the global scope. So we leave that here even though it is a bit duplicative.

jakirkham · 2022-02-26T00:58:09Z

recipe/meta.yaml

    - alimanfoo
    - jakirkham
    - jcrist
+    - jrbourbeau
    - koverholt
+    - marcelotrevisani
    - martindurant
    - mrocklin
+    - ogrisel
    - pitrou
+    - shoyer
+    - sinhrks
    - tomaugspurger


Merged all the packages' maintainers lists together into one sorted list

jakirkham · 2022-02-26T00:58:35Z

recipe/build-dask-core.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+
+pushd "${SRC_DIR}/dask"


cd into the particular source directory for the install

jakirkham · 2022-02-26T01:02:26Z

@jrbourbeau @jsignell would be good to get your thoughts on this 🙂

jsignell

Thanks for doing this @jakirkham! It looks pretty good to me. I just have some questions.

recipe/meta.yaml

jsignell · 2022-02-28T19:49:42Z

recipe/meta.yaml



 package:
-  name: dask
+  name: dask-split


Does the package need a name and if it does can the name just be dask?

Yeah unfortunately. It also can't duplicate one of the existing subpackages (have struggled with trying to do that before). Typically we just add -split here, but we could add anything as long as it doesn't duplicate other packages

That said, agree this is not the most ergonomic thing

README.md

jsignell · 2022-02-28T19:51:32Z

README.md


-Installing `dask` from the `conda-forge` channel can be achieved by adding `conda-forge` to your channels with:
+Installing `dask-split` from the `conda-forge` channel can be achieved by adding `conda-forge` to your channels with:


Even if we need a name for this recipe of outputs, I don't think dask-split should be referenced anywhere.

Yeah this text is autogenerated. Not too much we can do here.

Edit: Raised issue ( conda-forge/conda-smithy#1599 )

jsignell · 2022-02-28T19:52:35Z

README.md


 ```
-conda install dask
+conda install dask dask-core distributed


I think these should all just say dask since dask installs them all.

This is also autogenerated. So not much we can do here.

yeah ok I was kind of worried that was the case. I guess it's not too bad becuase it would be a pretty strange user who encountered this as readme as their install instructions. It a little bit seems like this is not the place for install instructions at all.

Well it seems reasonable to provide some kind of install instructions on the feedstock given this is where the recipe is maintained and packages published.

While this particular piece is duplicative in our case, it is not always so (there is not always a kitchen sink package to install) and it should still work (despite being a bit verbose).

The portion above with installing dask-split is a bit worse IMHO as that is actually providing inaccurate information that seems unlike to ever be correct.

That all being said, if you would like to raise a conda-smithy issue on how to improve this with some suggestions, would be supportive of that. Whenever conda-smithy has the intended improvement, it should be a simple re-render to replace what is currently here.

I'm commenting on the issue, but it looks like there might already be a way to specify a different package_name

Yeah I tried to use package_name at the time, but it didn't seem to solve the issue.

…nda-forge-pinning 2022.02.28.13.05.22 Now that `dask-core`'s `about/home` points to the webpage, re-render to fix the README.

Should fix some README templating.

jakirkham · 2022-02-28T20:24:23Z

Thanks Julia 🙂 Pushed some fixes based on feedback above

jsignell · 2022-03-15T14:28:45Z

It looks like there are some unmerged suggestions. Should we merge them in and give this a shot on friday?

jakirkham · 2022-03-15T14:30:38Z

Sorry not following. What unmerged suggestions?

jsignell · 2022-03-15T14:37:31Z

Sorry not following. What unmerged suggestions?

I commented on them, so hopefully that will help make them visible. I realized though that we probably can't merge this until the linter failure is fixed upstream.

conda-forge-linter · 2022-03-15T14:40:04Z

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

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

…nda-forge-pinning 2022.03.15.10.12.29

conda-forge-linter · 2022-03-15T14:55:51Z

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 (recipe) and found some lint.

Here's what I've got...

For recipe:

The outputs section contained an unexpected subsection name. entry_points is not a valid subsection name.

jakirkham · 2022-03-15T14:59:29Z

I commented on them, so hopefully that will help make them visible.

Thanks! Included those. Also fixed merge conflicts.

I realized though that we probably can't merge this until the linter failure is fixed upstream.

We can. We just have to accept a linter failure in the near term.

Am able to build locally with this change, but maybe there are other tooling issues we are missing by doing this.

…nda-forge-pinning 2022.03.15.10.12.29

jakirkham · 2022-03-15T15:16:07Z

recipe/meta.yaml

-  name: dask-split
+  name: dask


Are there any issues having a split package with the same name used here and in one of the outputs? In the past had issues with building, being converted by staged-recipes, etc. ( for example conda-forge/staged-recipes#7386 (comment) ), but am wondering how much of an issue this still is. For example this builds fine locally and was able to re-render. Thoughts? 🙂

cc @conda-forge/core

I just find that it is an edge case. So eventually, are you going to be waiting for it to fail because somebody has a bug in their code somewhere. Do you really want to play with that risk?

Naming the top-level package something like *-split has some effects that are undesirable ( #172 (review) ).

What else is the top-level package used for?

I have a pretty weak opinion on this front.

I do think that the autogenerated README should not be used as the end all documentation.

I do also think it is easier to wolve nits like this in conda smithy.

Same was mainly hoping to address the remaining reviewer comments. If we are wary of this, happy to go back to a different name here.

Please see this comment conda-forge/conda-smithy#1599 (comment). I'd appreciate it if you could try it before asking for more help.

This comment was marked as outdated.

Sign in to view

jakirkham force-pushed the use_split_pkgs branch from c2a2a5f to ce4fe31 Compare February 26, 2022 00:05

This comment was marked as outdated.

Sign in to view

jakirkham force-pushed the use_split_pkgs branch from ce4fe31 to 722b357 Compare February 26, 2022 00:06

jakirkham force-pushed the use_split_pkgs branch 6 times, most recently from c208d2b to 2148e84 Compare February 26, 2022 00:23

jakirkham force-pushed the use_split_pkgs branch 2 times, most recently from 5558bf6 to 38391f6 Compare February 26, 2022 00:32

jakirkham mentioned this pull request Feb 26, 2022

Lint of entry_points in outputs conda-forge/conda-smithy#1598

Closed

Use multiple outputs for dask & distributed

6888251

jakirkham force-pushed the use_split_pkgs branch from 38391f6 to 6888251 Compare February 26, 2022 00:46

MNT: Re-rendered with conda-build 3.21.8, conda-smithy 3.17.2, and co…

3df27cf

…nda-forge-pinning 2022.02.25.15.35.46

jakirkham commented Feb 26, 2022

View reviewed changes

jakirkham mentioned this pull request Feb 26, 2022

Create dask and dask-core packages using a single recipe conda-forge/dask-core-feedstock#59

Open

5 tasks

jsignell reviewed Feb 28, 2022

View reviewed changes

jakirkham added 5 commits February 28, 2022 12:08

Change number to build_number

0a84be5

Update about/home of dask-core

ae57b3e

MNT: Re-rendered with conda-build 3.21.8, conda-smithy 3.17.2, and co…

888ecc7

…nda-forge-pinning 2022.02.28.13.05.22 Now that `dask-core`'s `about/home` points to the webpage, re-render to fix the README.

Adjust conda-forge.yml's formatting

2ba81eb

Add github/repo_name

274a85a

Should fix some README templating.

Drop unneeded constraints from dask

7823066

jakirkham added 3 commits March 15, 2022 07:52

Drop duplicate summary

bbdd5df

MNT: Re-rendered with conda-build 3.21.8, conda-smithy 3.18.0, and co…

d833633

…nda-forge-pinning 2022.03.15.10.12.29

Merge conda-forge/main into jakirkham-feedstocks/use_split_pkgs

f9e23b8

jakirkham added 2 commits March 15, 2022 08:09

Try dropping -split

42b2efb

Am able to build locally with this change, but maybe there are other tooling issues we are missing by doing this.

MNT: Re-rendered with conda-build 3.21.8, conda-smithy 3.18.0, and co…

5ceaa43

…nda-forge-pinning 2022.03.15.10.12.29

jakirkham commented Mar 15, 2022

View reviewed changes

jakirkham mentioned this pull request May 20, 2024

Depend on dask-core not dask conda-forge/dask-image-feedstock#17

Open

1 task


		Installing `dask` from the `conda-forge` channel can be achieved by adding `conda-forge` to your channels with:
		Installing `dask-split` from the `conda-forge` channel can be achieved by adding `conda-forge` to your channels with:

Use multiple outputs for dask & distributed #172

Are you sure you want to change the base?

Use multiple outputs for dask & distributed #172

Conversation

jakirkham commented Feb 26, 2022

This comment was marked as outdated.

This comment was marked as outdated.

conda-forge-linter commented Feb 26, 2022

conda-forge-linter commented Feb 26, 2022

jakirkham commented Feb 26, 2022 • edited Loading

jakirkham 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

jakirkham Mar 16, 2022 • edited Loading

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

jakirkham commented Feb 26, 2022

jsignell 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

jakirkham Feb 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham Feb 28, 2022 • edited Loading

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

jakirkham commented Feb 28, 2022

jsignell commented Mar 15, 2022

jakirkham commented Mar 15, 2022

jsignell commented Mar 15, 2022

conda-forge-linter commented Mar 15, 2022

conda-forge-linter commented Mar 15, 2022

jakirkham commented Mar 15, 2022

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

Use multiple `outputs` for `dask` & `distributed` #172

Use multiple `outputs` for `dask` & `distributed` #172

jakirkham commented Feb 26, 2022 •

edited

Loading

jakirkham Mar 16, 2022 •

edited

Loading

jakirkham Feb 28, 2022 •

edited

Loading

jakirkham Feb 28, 2022 •

edited

Loading