add scipy-typed output #5

lucascolley · 2024-12-08T19:46:03Z

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-admin · 2024-12-08T19:47:22Z

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

I failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint --conda-forge . from the recipe directory. You can also examine the workflow logs for more detail.

_{This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12224737893. Examine the logs at this URL for more detail.}

conda-forge-admin · 2024-12-08T19:49:49Z

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/meta.yaml) and found it was in an excellent condition.

lucascolley · 2024-12-08T19:55:34Z

@conda-forge-admin, please rerender

conda-forge-admin · 2024-12-08T19:57:35Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you but ran into some issues. Please check the output logs of the GitHub Actions workflow below for more details. You can also ping conda-forge/core (using the @ notation) for further assistance or you can try rerendering locally.

The following suggestions might help debug any issues:

Is the recipe/{meta.yaml,recipe.yaml} file valid?
If there is a recipe/conda-build-config.yaml file in the feedstock make sure that it is compatible with the current global pinnnings.
Is the fork used for this PR on an organization or user GitHub account? Automated rerendering via the webservices admin bot only works for user GitHub accounts.

_{This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12224790519. Examine the logs at this URL for more detail.}

lucascolley · 2024-12-08T20:03:37Z

@h-vetinari any idea why I'm seeing

    script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
  ^^^^^^^^^^^^^^^^^^^^^^^^^
jinja2.exceptions.UndefinedError: 'PYTHON' is undefined

?

recipe/meta.yaml

jorenham · 2024-12-08T21:15:02Z

What was this for, again 🤔 ?

lucascolley · 2024-12-08T21:30:51Z

we're trying outputting scipy-typed from this feedstock, rather than the scipy one

jorenham · 2024-12-08T22:04:41Z

we're trying outputting scipy-typed from this feedstock, rather than the scipy one

hmm, I'm not sure what "outputting" means in this context 🤔

lucascolley · 2024-12-08T22:17:55Z

one feedstock can output (onto the conda-forge channel) multiple packages. Right now scipy-feedstock outputs both scipy and scipy-tests. The proposed scipy-typed package could be output from any feedstock (once there is approval from https://github.com/conda-forge/admin-requests). I initially thought it would make sense to output it from scipy-feedstock, as I was in the same mental model as pip extras, but it might be easier recipe-wise to output it from scipy-stubs-feedstock instead. The only real difference is who has permission to merge PRs. Also, new builds of scipy-typed would line-up with new builds of scipy-stubs rather than new builds of scipy.

jorenham · 2024-12-08T22:23:04Z

one feedstock can output (onto the conda-forge channel) multiple packages. Right now scipy-feedstock outputs both scipy and scipy-tests. The proposed scipy-typed package could be output from any feedstock (once there is approval from https://github.com/conda-forge/admin-requests). I initially thought it would make sense to output it from scipy-feedstock, as I was in the same mental model as pip extras, but it might be easier recipe-wise to output it from scipy-stubs-feedstock instead. The only real difference is who has permission to merge PRs. Also, new builds of scipy-typed would line-up with new builds of scipy-stubs rather than new builds of scipy.

Ok thanks for explaining; I can see how it makes more sense this way 👌🏻

h-vetinari · 2024-12-08T22:30:47Z

any idea why I'm seeing [...]

PYTHON only gets populated if there's a python dependency. You've moved the dependencies to the output, but haven't moved the build-script (and actually, you should move the python: noarch too, and add it to scipy-typed as well).

Note that conda-recipes have an implicit "global" output (named after package: name: ...), and if this coincides with an output name, this results in very unintuitive behaviour (conda/conda-build#4172). Standard fix is to rename the package-name into something not conflicting (e.g. scipy-stubs-split), and then add

 extra:
   recipe-maintainers:
     - lucascolley
     - jorenham
+  feedstock-name: scipy-stubs

Sidenote: you should also get rid of the templating on name - it's completely pointless (the package name cannot be changed), and creates both extra mental overhead plus more opportunities for bugs.

PS. You'll also need to rename the script if you want to run it in the output itself (using script: <xxx>.sh), because build.sh is a reserved name that will always be executed in the recipe-global build stage, which isn't doing anything here anymore.

PPS. There's a way to keep using the existing setup despite adding another output, but it's rather confusing, so I don't recommend doing it... It would entail adding a single-line output: scipy-typed (in this case explicitly repeating package: name: ...) like

outputs:
  - name: scipy-stubs  # nothing else!
  - name: scipy-typed
    [...]

but to actually test this output, you'd have to specify the test: section before outputs:.

lucascolley · 2024-12-08T22:46:03Z

thanks for the explanations @h-vetinari ! I'll try to address those suggestions.

Sidenote: you should also get rid of the templating on name - it's completely pointless (the package name cannot be changed), and creates both extra mental overhead plus more opportunities for bugs.

Makes sense 👍 pretty sure this is what grayskull generated for me.

conda-forge-admin · 2024-12-08T22:53:57Z

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

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

_{This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12225955566. Examine the logs at this URL for more detail.}

conda-forge-admin · 2024-12-09T00:27:53Z

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/meta.yaml) and found it was in an excellent condition.

lucascolley · 2024-12-09T00:29:32Z

from investigating locally, it looks like .mypyignore is not included in the sdist on PyPI

lucascolley · 2024-12-09T00:35:03Z

@jorenham perhaps we can add this?

[tool.hatch.build.targets.sdist]
include = [
    ".mypyignore",
]

It seems like hatch ignores dotfiles by default.

lucascolley · 2024-12-09T01:11:24Z

I think I've made some progress, but I'd appreciate some further guidance @h-vetinari !

recipe/meta.yaml

jorenham · 2024-12-09T11:06:14Z

include = [
    ".mypyignore",
]

That doesn't seem to work 🤔

jorenham · 2024-12-09T11:11:23Z

include = [
    ".mypyignore",
]
That doesn't seem to work 🤔

this works though

[tool.hatch.build.targets.sdist.force-include]
".mypyignore" = ".mypyignore"

jorenham · 2024-12-10T02:13:56Z

There's a bug in stubtest that's causing it to crash with numpy==2.2.0, see jorenham/scipy-stubs#288 for the exact error in scipy-stubs, and KotlinIsland/basedmypy#842 the proposed fix in stubtest

lucascolley · 2024-12-15T23:42:56Z

locally I now see

+ stubtest --mypy-config-file=pyproject.toml --allowlist=.mypyignore --ignore-unused-allowlist scipy
error: scipy._lib.test_conda_forge_packaging failed to find stubs

which comes from https://github.com/conda-forge/scipy-feedstock/blob/9fe7f7450dc83b1b366dfafc3a335238103d1951/recipe/build-output.sh#L4-L9.

I guess we just want to add that to .mypyignore?

jorenham · 2024-12-15T23:43:12Z

There's a bug in stubtest that's causing it to crash with numpy==2.2.0, see jorenham/scipy-stubs#288 for the exact error in scipy-stubs, and KotlinIsland/basedmypy#842 the proposed fix in stubtest

In the meantime the fix for this has been released in the new 2.8.1 version of basedmypy

jorenham · 2024-12-15T23:46:42Z

I guess we just want to add that to .mypyignore?

I guess? It's probably cleanest to put it in a another one though (you can pass multiple --allowlist)

lucascolley · 2024-12-15T23:55:36Z

(you can pass multiple --allowlist)

that is convenient 😄 let's see if this passes

lucascolley · 2024-12-16T00:12:03Z

@conda-forge-admin, please rerender

conda-forge-admin · 2024-12-16T00:14:25Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

_{This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12343479817. Examine the logs at this URL for more detail.}

recipe/meta.yaml

.ci_support/linux_64_.yaml

lucascolley · 2024-12-16T12:10:11Z

@conda-forge-admin, please rerender

…onda-forge-pinning 2024.12.15.16.05.58

lucascolley

thanks @h-vetinari , addressed

README.md

recipe/meta.yaml

h-vetinari · 2024-12-16T12:40:08Z

recipe/.mypyignore-extra

@@ -0,0 +1 @@
+scipy\._lib\.test_conda_forge_packaging


FWIW, I'll happily take PRs that make this test compatible with whatever basedmypy is doing.

hmm, the script does import scipy.fft.tests - should mypy pick up the stubs from scipy-stubs @jorenham ? If so, maybe this would work?

import pytest + def test_output_separation() -> NoReturn: - def test_output_separation(): need_to_warn = False try: # check if we can import a test folder that's deleted in `scipy` # but present in `scipy-tests`; if this passes, skip the test import scipy.fft.tests pytest.skip("Can be ignored when `scipy-tests` is installed") except ModuleNotFoundError: # don't re-raise directly to reduce stacktrace, i.e. avoid # "During handling of the above exception, another exception occurred:" need_to_warn = True if need_to_warn: raise ImportError( "conda-forge builds of `scipy` do not package the tests by default " "to reduce the package footprint; you can ensure they're present by " "installing `scipy-tests` (resp. adding that to your dependencies)." )

Hmm not sure what you mean @lucascolley 🤔

stubtest complains about no stubs for the script which scipy-feedstock adds, so instead of ignoring the file could we also annotate it inline? Or give it its own stub file? At first I wasn't sure whether it would pick up stubs for the import from scipy, but I think it should given that we have scipy-stubs installed at test-time.

either way that's a minor point and doesn't need to block this from being merged.

Ah yea, sure. In that case, it should probably return -> None, as it doesn't always raise.

And I agree that stubbing it instead of ignoring it would a nice flex

lucascolley · 2024-12-16T12:49:56Z

@conda-forge-admin, please rerender

conda-forge-admin · 2024-12-16T12:52:04Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

_{This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12353013891. Examine the logs at this URL for more detail.}

lucascolley · 2024-12-18T11:30:53Z

@h-vetinari are you okay with me merging this now? I think it would be nice to be able to check that this all works before 1.15.0 is released.

lucascolley · 2024-12-18T13:13:14Z

thanks both!

jorenham · 2024-12-18T13:34:39Z

Thanks @lucascolley ❤️

add scipy-typed output

c3a4578

fix typo

f8debdf

jorenham reviewed Dec 8, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

address review comments

348452f

wip

1291b5c

lucascolley added 2 commits December 9, 2024 00:46

patch in mypyignore

b4cdf8a

add build script?

6065d08

h-vetinari reviewed Dec 9, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

move script: one level up

1e4933c

h-vetinari reviewed Dec 9, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

h-vetinari reviewed Dec 9, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

Merge branch 'main' into scipy-typed

3014194

add .mypyignore-extra

f2d1bca

not an empty file...

0f60173

lucascolley marked this pull request as ready for review December 15, 2024 23:58

lucascolley requested a review from h-vetinari December 15, 2024 23:58

h-vetinari requested changes Dec 16, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

.ci_support/linux_64_.yaml Outdated Show resolved Hide resolved

cfep-25

3767302

MNT: Re-rendered with conda-build 24.11.2, conda-smithy 3.45.0, and c…

9295671

…onda-forge-pinning 2024.12.15.16.05.58

lucascolley commented Dec 16, 2024

View reviewed changes

README.md Show resolved Hide resolved

lucascolley requested a review from h-vetinari December 16, 2024 12:16

h-vetinari reviewed Dec 16, 2024

View reviewed changes

recipe/meta.yaml Show resolved Hide resolved

h-vetinari reviewed Dec 16, 2024

View reviewed changes

add python test requirement

14a891a

lucascolley requested a review from h-vetinari December 16, 2024 13:45

h-vetinari approved these changes Dec 18, 2024

View reviewed changes

lucascolley merged commit fc110f7 into conda-forge:main Dec 18, 2024
5 checks passed

lucascolley deleted the scipy-typed branch December 19, 2024 00:02

		@@ -0,0 +1 @@
		scipy\._lib\.test_conda_forge_packaging

add scipy-typed output #5

add scipy-typed output #5

Conversation

lucascolley commented Dec 8, 2024 • edited Loading

conda-forge-admin commented Dec 8, 2024

conda-forge-admin commented Dec 8, 2024

lucascolley commented Dec 8, 2024

conda-forge-admin commented Dec 8, 2024

lucascolley commented Dec 8, 2024

jorenham commented Dec 8, 2024

lucascolley commented Dec 8, 2024

jorenham commented Dec 8, 2024

lucascolley commented Dec 8, 2024 • edited Loading

jorenham commented Dec 8, 2024

h-vetinari commented Dec 8, 2024

lucascolley commented Dec 8, 2024

conda-forge-admin commented Dec 8, 2024

conda-forge-admin commented Dec 9, 2024

lucascolley commented Dec 9, 2024

lucascolley commented Dec 9, 2024

lucascolley commented Dec 9, 2024

jorenham commented Dec 9, 2024

jorenham commented Dec 9, 2024

jorenham commented Dec 10, 2024

lucascolley commented Dec 15, 2024

jorenham commented Dec 15, 2024

jorenham commented Dec 15, 2024 • edited Loading

lucascolley commented Dec 15, 2024

lucascolley commented Dec 16, 2024

conda-forge-admin commented Dec 16, 2024

lucascolley commented Dec 16, 2024

lucascolley left a comment

Choose a reason for hiding this comment

h-vetinari Dec 16, 2024

Choose a reason for hiding this comment

lucascolley Dec 16, 2024

Choose a reason for hiding this comment

jorenham Dec 16, 2024

Choose a reason for hiding this comment

lucascolley Dec 16, 2024

Choose a reason for hiding this comment

lucascolley Dec 16, 2024

Choose a reason for hiding this comment

jorenham Dec 16, 2024

Choose a reason for hiding this comment

jorenham Dec 16, 2024

Choose a reason for hiding this comment

lucascolley commented Dec 16, 2024

conda-forge-admin commented Dec 16, 2024

lucascolley commented Dec 18, 2024

lucascolley commented Dec 18, 2024

jorenham commented Dec 18, 2024

lucascolley commented Dec 8, 2024 •

edited

Loading

lucascolley commented Dec 8, 2024 •

edited

Loading

jorenham commented Dec 15, 2024 •

edited

Loading