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

Attempt to fix some URL bugs #525

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Attempt to fix some URL bugs #525

wants to merge 2 commits into from

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Feb 1, 2024

Description

I made a quick attempt to fix #447, but it's not working yet. Output looks like:

{% set name = "pyospackage" %}
{% set version = "0.1.10" %}

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

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

build:
  noarch: python
  script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
  number: 0

requirements:
  host:
    - python >=3.9
    - hatchling
    - pip
  run:
    - python >=3.9

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

about:
  home: https://pypi.org/project/pyospackage/
  summary: A package that adds numbers together
  doc_url: https://github.com/pyopensci/pyospackage
 # readme
  dev_url: https://github.com/pyopensci/pyospackage
 # readme
  license: MIT
 # readme
  license_file: LICENSE  # readme

extra:
  recipe-maintainers:
    - maresb

I'm not sure where those # readme comments are coming from, and I'm not sure why the doc_url gets clobbered during the rendering. (It's correct when about_section is defined.)

@marcelotrevisani
Copy link
Member

yeah, tricky, it comes from the pypi json payload
Grayskull also tries to fetch info from there, for some reason is putting a breakline after the end of the URL
this is part of the json response from pypi for that package:

"project_url": "https://pypi.org/project/pyospackage/",
    "project_urls": {
      "Documentation": "https://github.com/pyopensci/pyospackage#readme",
      "Homepage": "https://pypi.org/project/pyospackage/",
      "Issues": "https://github.com/pyopensci/pyospackage/issues",
      "Source": "https://github.com/pyopensci/pyospackage"
    },

can you see that #readme at the end of Documentation? it is coming from there. I hope that helps, if you need more help with that please let me know :)

@maresb
Copy link
Contributor Author

maresb commented Feb 1, 2024

Woah, thanks a lot @marcelotrevisani for the advice and quick response! Not sure when I'll be able to make a second pass at this. Looks like maybe something needs to be quoted/escaped?

@marcelotrevisani
Copy link
Member

Woah, thanks a lot @marcelotrevisani for the advice and quick response! Not sure when I'll be able to make a second pass at this. Looks like maybe something needs to be quoted/escaped?

Yeah, probably it does need some escaping. I can try to take a look later to give you a hint about that

@msarahan
Copy link
Contributor

msarahan commented Feb 1, 2024

@maresb keep in mind that a lot of the code in play here is part of souschef

@lwasser
Copy link

lwasser commented Feb 6, 2024

@msarahan should we submit a bug report to souchef repository as well? i'm just watching this. it would be nice to fix this because otherwise users will get that linting error when they submit their recipe and need to figure out how to fix things.

@maresb
Copy link
Contributor Author

maresb commented Feb 6, 2024

I think that there are two stacked bugs here.

One bug has to do with the metadata getting passed around without being accidentally dropped. The other has to do with the # in the URL being interpreted as a comment.

I actually think that I fixed the bug with the metadata being dropped, but unmasking the bug due to the # in @lwasser's URL.

@lwasser, as a temporary workaround could you try removing the #... part? I'm thinking that it may be easier to proceed with one bug at a time.

@maresb
Copy link
Contributor Author

maresb commented Feb 6, 2024

Restarting the CI due to the connection error...

@maresb maresb closed this Feb 6, 2024
@maresb maresb reopened this Feb 6, 2024
@msarahan
Copy link
Contributor

msarahan commented Feb 6, 2024

It has seemed like @maresb has this covered. Ben, if you need my help, please ping me. Otherwise, I'm leaving it in your capable hands and cheering you on.

@lwasser
Copy link

lwasser commented Feb 6, 2024

ok great! thanks @msarahan !!
@maresb can you remind me - what link are you referring to with the commented out element? i'm happy to test against this branch but i'm unclear about what you have identified that i can modify. many thanks! ❇️

@maresb
Copy link
Contributor Author

maresb commented Feb 6, 2024

@lwasser it's your docs link: https://github.com/pyopensci/pyospackage#readme. I think the current PR would fix the recipe if your docs link were instead simply https://github.com/pyopensci/pyospackage.

@lwasser
Copy link

lwasser commented Feb 7, 2024

@lwasser it's your docs link: https://github.com/pyopensci/pyospackage#readme. I think the current PR would fix the recipe if your docs link were instead simply https://github.com/pyopensci/pyospackage.

Thank you. i see that now. @maresb in testing this can i run grayskull against a sdist locally so i can skip releasing another version to pypi? i keep digging through the docs but don't see that option (may be user error)?

@msarahan
Copy link
Contributor

msarahan commented Feb 7, 2024

You can, but you'll need to adjust the source section.

From README.md:

You can also generate a recipe from a local sdist archive:

grayskull pypi ./pytest-5.3.5.tar.gz

Note that such a recipe isn't really portable as it will depend on the local path of the sdist file. It can be useful if you want to automatically generate a conda package.

@lwasser
Copy link

lwasser commented Feb 7, 2024

Thank you everyone @msarahan i missed that in the readme. i was checking the docs. As a side note would folks here be open to a documentation pr from me that had the get started info in the readme? it was confusing to me as i saw docs and assumed that info would be there rather than in the readme! I suppose the alternative coming with less maintenance would be to just include the readme in the docs! just a thought :)

@maresb Ok i was not understanding the issue and now i see it. There is no reason to have a # in a url as you point out. when i remove that my metadata render perfectly (from your branch) with grayskull running in editable mode on my computer.

The metadata / recipe now look like this:

{% set name = "pyospackage" %}
{% set version = "0.1.10" %}

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

source:
  url: file:///Users/leahawasser/Documents/GitHub/pyos/pyospackage/dist/pyospackage-0.1.10.tar.gz
  sha256: 1db1ceb1d769a081c2945d235016ddb6dfe1b555c68014704c978b198b841ded

build:
  noarch: python
  script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
  number: 0

requirements:
  host:
    - python >=3.9
    - hatchling
    - pip
  run:
    - python >=3.9

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

about:
  home: https://pypi.org/project/pyospackage/
  summary: A package that adds numbers together
  license: MIT
  license_file: LICENSE

extra:
  recipe-maintainers:
    - lwasser

i wonder if this pr could be merged as i am not seeing anything else broken here in the output. thank you all for walking me through what was going on. i think i misunderstood above what "breaking things" related to.

@@ -401,11 +401,36 @@ def get_metadata(recipe, config) -> dict:
test_requirements = optional_requirements.pop(config.extras_require_test, [])
test_section = compose_test_section(metadata, test_requirements)

# Compute home, doc_url, and dev_url for the "about" section

if metadata.get("project_urls") and metadata["project_urls"].get("Homepage"):
Copy link
Member

Choose a reason for hiding this comment

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

can you encapsulate all this handling for the URLs in a function please?
so we can test that easily :)

@marcelotrevisani
Copy link
Member

Thank you everyone @msarahan i missed that in the readme. i was checking the docs. As a side note would folks here be open to a documentation pr from me that had the get started info in the readme? it was confusing to me as i saw docs and assumed that info would be there rather than in the readme! I suppose the alternative coming with less maintenance would be to just include the readme in the docs! just a thought :)

@maresb Ok i was not understanding the issue and now i see it. There is no reason to have a # in a url as you point out. when i remove that my metadata render perfectly (from your branch) with grayskull running in editable mode on my computer.

The metadata / recipe now look like this:

{% set name = "pyospackage" %}
{% set version = "0.1.10" %}

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

source:
  url: file:///Users/leahawasser/Documents/GitHub/pyos/pyospackage/dist/pyospackage-0.1.10.tar.gz
  sha256: 1db1ceb1d769a081c2945d235016ddb6dfe1b555c68014704c978b198b841ded

build:
  noarch: python
  script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
  number: 0

requirements:
  host:
    - python >=3.9
    - hatchling
    - pip
  run:
    - python >=3.9

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

about:
  home: https://pypi.org/project/pyospackage/
  summary: A package that adds numbers together
  license: MIT
  license_file: LICENSE

extra:
  recipe-maintainers:
    - lwasser

i wonder if this pr could be merged as i am not seeing anything else broken here in the output. thank you all for walking me through what was going on. i think i misunderstood above what "breaking things" related to.

I'm glad that this is sorted out, but that is still a bug that I can fix it later, because it should be able to handle the # fine. It seems something related to the parsing and ruamel.yaml
I have been quite busy these days with my day work and personal life that I didn't have much time to look into it, but I hope to fix this soon :)

Thank you everybody for putting efforts into this.

@maresb , would you like to change the status of this PR to "ready for review"? or are you going to work more in this front?

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.

[BUG] about.home defaults to None instead of PyPI project page
4 participants