Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: allow specifying a list of values in context #1289

Open
mgorny opened this issue Dec 27, 2024 · 0 comments
Open

Feature request: allow specifying a list of values in context #1289

mgorny opened this issue Dec 27, 2024 · 0 comments

Comments

@mgorny
Copy link
Contributor

mgorny commented Dec 27, 2024

I would like to request the ability to specify a list values in context rather than just a scalar. This is a bit related to #971, but that bug seems more focused on the Jinja expression being flattened, while I'm interested in ability to use YAML lists.

For example, torchvision-feedstock originally skipped tests like this:

    # skip test_url_is_accessible instead of hitting 20+ servers per run, since
    # each server might be occasionally unresponsive and end up failing our CI
    {% set tests_to_skip = "test_url_is_accessible" %}
    # spurious failure because upstream skip (Image.__version__ >= "7") does not trigger for Pillow "10"
    {% set tests_to_skip = tests_to_skip + " or (test_transforms and test_adjust_saturation)" %}
    # osx warns with nnpack if there is no AVX2, see conda-forge/pytorch-cpu-feedstock#56
    {% set tests_to_skip = tests_to_skip + " or test_adjust_sharpness" %}  # [osx]

which is quite cumbersome, but permits including comments.

What I'd ideally like to be able it to specify:

context:
  tests_to_skip:
    # skip test_url_is_accessible instead of hitting 20+ servers per run, since
    # each server might be occasionally unresponsive and end up failing our CI
    - test_url_is_accessible
    # spurious failure because upstream skip (Image.__version__ >= "7") does not trigger for Pillow "10"
    - (test_transforms and test_adjust_saturation)
    # osx warns with nnpack if there is no AVX2, see conda-forge/pytorch-cpu-feedstock#56
    - if: osx
      then: test_adjust_sharpness

that would create a Jinja list that I could afterwards combine via ${{ tests_to_skip | join(" or ") }} or likewise.

I suppose getting conditions in there may be problematic, but even without that part, I think the feature would be really helpful.

For contrast, right now I'm working around the problem via:

context:
  tests_to_skip: >
    ${{ 'skip test_url_is_accessible instead of hitting 20+ servers per run, since' if 0 }}
    ${{ 'each server might be occasionally unresponsive and end up failing our CI' if 0 }}
    test_url_is_accessible
    ${{ 'spurious failure because upstream skip (Image.__version__ >= "7") does not trigger for Pillow "10"' if 0 }}
    or (test_transforms and test_adjust_saturation)
    ${{ 'osx warns with nnpack if there is no AVX2, see conda-forge/pytorch-cpu-feedstock#56' if 0 }}
    ${{ "or test_adjust_sharpness" if osx }}

and while it serves the primary purpose (having comments in place), it is kinda ugly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant