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

[BUG] meta.yaml order is not in correct order sometimes #288

Closed
BastianZim opened this issue Feb 1, 2022 · 17 comments · Fixed by #355
Closed

[BUG] meta.yaml order is not in correct order sometimes #288

BastianZim opened this issue Feb 1, 2022 · 17 comments · Fixed by #355
Labels
bug Something isn't working

Comments

@BastianZim
Copy link
Contributor

BastianZim commented Feb 1, 2022

Describe the bug
A clear and concise description of what the bug is.

Ran grayskull on awswrangler and the order is not correct.

To Reproduce
Steps to reproduce the behavior:

  1. Run grayksull on awswrangler

Expected behavior
A clear and concise description of what you expected to happen.

Source should be after package and before build.

Outputs
If applicable, add the output to help explain your problem.

Example output:

{% set name = "requests" %}
{% set version = "2.27.1" %}

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

build:
  skip: true  # [py==30 or py==31 or py==32 or py==33 or py==34 or py==35]
  script: {{ PYTHON }} -m pip install . -vv
  number: 0

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

requirements:
  host:
    - pip
    - pytest
    - python
  run:
    - certifi >=2017.4.17
    - chardet >=3.0.2,<5  # [py<3]
    - charset-normalizer >=2.0.0  # [py>=3],==2.0.*
    - idna >=2.5,<3  # [py<3]
    - python
    - urllib3 >=1.21.1,<1.27

test:
  imports:
    - requests
  commands:
    - pip check
  requires:
    - pip

about:
  home: https://requests.readthedocs.io
  summary: Python HTTP for Humans.
  dev_url: https://github.com/psf/requests
  license: Apache-2.0
  license_file: LICENSE

extra:
  recipe-maintainers:
    - ADD_YOUR_GITHUB_ID_HERE

Environment:

https://www.marcelotrevisani.com/grayskull

Additional context
Add any other context about the problem here.

  • This only started happening recently.
  • I'm not sure if this is really an urgent issue but to maintain readability it might be nice to use the original sorting.
  • This doesn't always happen.
@BastianZim BastianZim added the bug Something isn't working label Feb 1, 2022
@BastianZim BastianZim changed the title [BUG] meta.yaml order is not correct. [BUG] meta.yaml order is not in correct order sometimes Feb 1, 2022
@marcelotrevisani
Copy link
Member

Hi, thanks for reporting it :)

I would like to understand what do you mean by the original sorting? do you want it to be alphabetically sorted?
I believe it was a request of @jakirkham to preserve the original order of the metadata.
I believe I can add an option to sort it alphabetically then, what do you think?
something like ``--sort-deps```

@BastianZim
Copy link
Contributor Author

BastianZim commented Feb 4, 2022

Ahh sorry I mean the actual meta.yaml itself.

Normally it's package, source, build but sometimes it's package, build, source and I'm not sure why.

@ForgottenProgramme
Copy link
Contributor

Wow.
This is weird.

@BastianZim
Copy link
Contributor Author

@marcelotrevisani I haven't seen it happening often but every now and then. Let me know if I should start a list of packages where this occurs.

@marcelotrevisani
Copy link
Member

I think I know what is happening but if you have more packages as examples to test it would be really nice, thanks! :)

@BastianZim
Copy link
Contributor Author

BastianZim commented Feb 16, 2022

Just starting a list here and will update when I notice packages:

  • galeodes
  • dimod
  • drm4g
  • deepface

@BastianZim
Copy link
Contributor Author

@marcelotrevisani Sorry, I forgot to update the list at some point but this is still happening. Do you have a clearer idea about how to fix this maybe? It's not super big, was just wondering. :)

@ForgottenProgramme
Copy link
Contributor

#333

@ForgottenProgramme
Copy link
Contributor

I tried the packages @BastianZim mentioned. This was the result;
awswrangler : correct order
galeodes : correct order
drm4g : wrong order
deepface: wrong order
requests: wrong order

I notice the order of source and build is wrong in recipes where the build section has a skip: true line.

@marcelotrevisani
Copy link
Member

Indeed, that is not correct. I have an idea what is happening.
I just don't have much time these days, actually, until July, I will not have much time to take a look on this :/
But I will try, just can't promise enough.

The thing is, the ruamel yaml creates a dict like object and it preserves the order when you create new keys and it is difficult to change the order after the creation of new keys that are not in the correct order. The strategy to workaround this is to create all sections that grayskull could use at the begining of the creation of the recipe object, and instanciate that with a key and an empty value. If the value is empty at the end before to dump the values to the file, grayskull try to clean the recipe.

@BastianZim
Copy link
Contributor Author

Ahh no rush from my side :) I was just wondering if you have an idea about what's happening so someone else can take a look.

@sgbaird
Copy link
Contributor

sgbaird commented May 26, 2022

Might be worth mentioning the order change is not compatible with conda-forge recipes: conda-forge/staged-recipes#19083 (comment)

@sgbaird
Copy link
Contributor

sgbaird commented Jun 9, 2022

I have a conda-souschef workaround at marcelotrevisani/souschef#51 (comment)

@BastianZim
Copy link
Contributor Author

That’s great! :) Can you make a PR there maybe?

@sgbaird
Copy link
Contributor

sgbaird commented Jun 9, 2022

@BastianZim thanks! On conda-souschef you mean? Assuming so, are you thinking a default behavior of my_recipe = Recipe.load(load_file=fpath), something that gets called after the fact my_recipe.sort() or something else? my_recipe.sort() method seems like a good option to me.

@BastianZim
Copy link
Contributor Author

To be honest, that’s probably better addressed at @marcelotrevisani
I don‘t know this codebase at all so I was just hoping that you maybe have a solution. :)

@marcelotrevisani
Copy link
Member

Yeah, that needs to be addressed at conda-souschef, indeed

Sorry for my delay, I was off for the past 2 weeks because of some personal matters. But I am back now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants