conda-forge / nbconvert-feedstock

A conda-smithy repository for nbconvert.
BSD 3-Clause "New" or "Revised" License
4 stars 24 forks source link

Update meta.yaml - add pandoc max version #94 #95

Closed achimgaedke closed 1 year ago

achimgaedke commented 1 year ago

Checklist

implements fix for #94 - did not bumb the build number as I would belive it is not high priority.

conda-forge-webservices[bot] commented 1 year 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.

bollwyvl commented 1 year ago

Good find. Ideally this would add a test that actually would hit that version check, likely more robust than trying to parse it out from wherever the problem is.

bollwyvl commented 1 year ago

Also, as we have the split packages, we have the opportunity to test/requres against both the oldest-supported-pandoc and the latest-supported-pandoc. This won't find if the top end has moved upstream, however, but would at least tell us whether the range is a strict subset of the expected versions.

The real win would be if the upstream had this information in a trivially-parseable file (e.g. pyproject.toml), or some CLI to print the expected bounds.

achimgaedke commented 1 year ago

Would be https://conda.io/projects/conda-build/en/stable/resources/define-metadata.html#loading-data-from-other-files the right mechanism for it?

{% set min_version = load_file_regex(
    load_file='nbconvert/utils/pandoc.py',
    regex_pattern=r'^_minimal_version\s*=\s*"(\S+)"') %}

and the same for the maximal version, add both strings using jinja to the appropriate pandoc requirements line.

Following the comments in meta.yaml, line 80 seemed to be the correct position.

achimgaedke commented 1 year ago

As pandoc is not a python/pip package the requirements are not formalized in any other file. Managing the requirements in this case is the reason for (re-)package managers like conda-forge. 😸

bollwyvl commented 1 year ago

Using "fancy" Jinja for requirements breaks some of the bot features, so it won't even get prs: tests are generally more reliable, and hard coded values are easier to track when things change.

For example:

- python -c "p = __import__('nbconvert. pandoc').pandoc; assert

(p._min_pandoc, p._max_pandoc)==('{{ min_pandoc }},'{{ max_pandoc }}')"

achimgaedke commented 1 year ago

That makes sense, unfortunately templates were the first thing I stumbled over in the docs. I'm curious to see the worked-out solution, I'm offline for some days.

achimgaedke commented 1 year ago

Ah, just saw https://github.com/conda-forge/nbconvert-feedstock/pull/96 - but already started to build "my own"... should have read the thread more carefully.

Awesome to see it grow... @bollwyvl has a separate python test file instead of an unwieldy command line -> #96 is better