conda-incubator / conda-recipe-manager

A project for libraries and automated tools that manage and manipulate conda recipe files.
BSD 3-Clause "New" or "Revised" License
10 stars 8 forks source link

pywavelets-feedstock failed to parse #229

Open rgommers opened 2 weeks ago

rgommers commented 2 weeks ago

What happened?

Trying conda-recipe-manager convert on https://github.com/conda-forge/pywavelets-feedstock/ resulted in an obscure error message:

$ conda-recipe-manager convert ./recipe/meta.yaml
EXCEPTION: An exception occurred while parsing the recipe file
pop from empty list
0 errors and 0 warnings were found.

To Reproduce

Steps to reproduce the behavior:

  1. git clone git@github.com:conda-forge/pywavelets-feedstock.git
  2. cd pywavelets-feedstock
  3. conda-recipe-manager convert ./recipe/meta.yaml

Additional Details (please complete the following information):

schuylermartin45 commented 2 weeks ago

Thank you for reporting this! I can't find an existing ticket for this error.

I haven't tested this, but looking at the recipe file, my guess is that the issue lies with:

    - pip check
    {% set label = "'full'" %}
    {% set tests = "['pywt']" %}
    - python -c "import sys; import pywt; sys.exit(not pywt.test(verbose=2, label={{ label }}, tests={{ tests }}, extra_argv=['--durations=50', '-k', 'not (test_mra_axis or test_mra2_axes or test_mra2_roundtrip or test_iswtn_mixed_dtypes)']))"

Those variables are already set at the top of the file, so I assume they are safe to remove here on lines 52-53.

If that doesn't solve the issue, I don't think the parser currently supports JINJA list variables. That might take some time to implement. In my experience, it is relatively uncommon to see that in a recipe file, so it hasn't come up yet.

In the mean time, you could attempt to comment/delete those lines and add them back in, post-conversion. That should at least alleviate some of the manual conversion effort.

I'm going to capture the file here for ease of discovery, if those suggestions end up working for you.

{% set label = "'full'" %}
{% set tests = "['pywt']" %}
{% set name = "pywavelets" %}
{% set version = "1.7.0" %}

package:
  name: {{ name|lower }}
  version: {{ version }}

source:
  url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
  sha256: b47250e5bb853e37db5db423bafc82847f4cde0ffdf7aebb06336a993bc174f6

build:
  number: 2
  skip: true  # [py<310]
  script:
    - mkdir builddir
    - {{ PYTHON }} -m mesonbuild.mesonmain setup $MESON_ARGS builddir   # [unix]
    - {{ PYTHON }} -m mesonbuild.mesonmain setup %MESON_ARGS% builddir  # [win]
    - {{ PYTHON }} -m build --wheel --no-isolation --skip-dependency-check -Cbuilddir=builddir
    - {{ PYTHON }} -m pip install --find-links dist pywavelets

requirements:
  build:
    - python                                 # [build_platform != target_platform]
    - cross-python_{{ target_platform }}     # [build_platform != target_platform]
    - cython >=3.0.4                         # [build_platform != target_platform]
    - numpy                                  # [build_platform != target_platform]
    - ninja                                  # [unix]
    - {{ compiler('c') }}
    - {{ stdlib('c') }}
  host:
    - python
    - python-build
    - pip
    - cython >=3.0.4
    - numpy
    - meson-python >=0.16.0
  run:
    - numpy >=1.23,<3
    - python

test:
  requires:
    - pytest
    - pip
  imports:
    - pywt
  commands:
    - pip check
    {% set label = "'full'" %}
    {% set tests = "['pywt']" %}
    - python -c "import sys; import pywt; sys.exit(not pywt.test(verbose=2, label={{ label }}, tests={{ tests }}, extra_argv=['--durations=50', '-k', 'not (test_mra_axis or test_mra2_axes or test_mra2_roundtrip or test_iswtn_mixed_dtypes)']))"

about:
  home: https://github.com/PyWavelets/pywt
  license: MIT
  license_file: LICENSE
  summary: Discrete Wavelet Transforms in Python

extra:
  recipe-maintainers:
    - grlee77
    - jakirkham
    - ocefpaf
    - rgommers
rgommers commented 2 weeks ago

Those variables are already set at the top of the file, so I assume they are safe to remove here on lines 52-53.

That is indeed the issue, thanks!

In my experience, it is relatively uncommon to see that in a recipe file, so it hasn't come up yet.

The same issue is present also in the numpy and scipy feedstocks, so I suspect there's at least one fairly prolific recipe author who made a habit of using this pattern.

schuylermartin45 commented 2 weeks ago

The same issue is present also in the numpy and scipy feedstocks, so I suspect there's at least one fairly prolific recipe author who made a habit of using this pattern.

Thanks for calling that out! If there are enough cases of this, there is a pre-processor stage in the conversion process that we could leverage to delete bad lines before we attempt to parse the recipe file.

You would not believe the number of common spelling mistakes for recipe fields I've already had to correct for :)

rgommers commented 2 weeks ago

Changing {% set ... lines in lists to # FIXME:{% set ...` would probably be helpful to do indeed. Fixing up the recipe is easy, but the root cause can't be found from the error message right now.