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

Depend on upstream ruamel.yaml instead of unmaintained ruamel_yaml #291

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mbargull
Copy link
Member

@mbargull mbargull commented Nov 1, 2023

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.

I'm currently looking at what packages on conda-forge have a dependency on the non-maintained ruamel_yaml package.

@mbargull mbargull requested a review from a team as a code owner November 1, 2023 12:07
@conda-forge-webservices
Copy link
Contributor

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.

Comment on lines +12 to +20
def get_yaml_safe():
# define global yaml API
# safe-loader and allowing duplicate keys
# for handling # [filter] / # [not filter]
# Don't use a global variable for this as a global
# variable will make conda-smithy thread unsafe.
yaml = YAML(typ="safe")
yaml.allow_duplicate_keys = True
return yaml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just copy-pasta from conda-smithy.utils.get_yaml with roundtrip-loader exchanged with safe-loader.
YAML requires unique keys, but PyYAML only warns instead of raising errors; allow_duplicate_keys = True is for compatibility with PyYAML. I don't know if we need it here though (IMO, error would be better).

recipe/meta.yaml Outdated
@@ -73,7 +73,7 @@ requirements:
- shyaml
- jq # [unix]
- requests
- ruamel_yaml
- ruamel.yaml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ruamel.yaml
- ruamel.yaml <0.19.0a0

would probably make sense. (No idea about what lower bound would be needed.)

Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
…nda-forge-pinning 2023.11.10.18.46.05

Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
@@ -1,5 +1,5 @@
{% set name = "conda-forge-ci-setup" %}
{% set version = "4.1.0" %}
{% set version = "4.1.1" %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% set version = "4.1.1" %}
{% set version = "4.3.4" %}

@jaimergp
Copy link
Member

jaimergp commented Apr 3, 2024

@mbargull This LGTM. Should we try to land it?

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

Successfully merging this pull request may close these issues.

2 participants