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

Add xyzspaces #12501

Merged
merged 7 commits into from
Aug 31, 2020
Merged

Add xyzspaces #12501

merged 7 commits into from
Aug 31, 2020

Conversation

deeplook
Copy link
Contributor

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml"
  • License file is packaged (see here for an example)
  • Source is from official source
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged)
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details)
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there

@conda-forge-linter
Copy link

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 (recipes/xyzspaces) and found it was in an excellent condition.

@deeplook
Copy link
Contributor Author

Since I am posting this PR I am also willing to be listed here... ;)

@deeplook
Copy link
Contributor Author

deeplook commented Aug 28, 2020

I see in the Azure pipeline logs that fiona is used, but not declared as a requirement. Which is weird because on Travis-CI there this is building fine. Maybe the conda builds are in some sense stricter, and if so I'd like to know in which one?


build:
number: 0
skip: True # [py<36]
Copy link
Member

Choose a reason for hiding this comment

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

conda-forge isn't building for pytho<3.6, so you can remove the skip and use noarch: python:

https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-python

you can always add the version restrictions under requirements (i.e change python to python >=3.6 under host and run)

- python
- pip
run:
- python
Copy link
Member

Choose a reason for hiding this comment

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

You need to add everything in the setup.py's install_requires, which looks to be read out of:

https://github.com/heremaps/xyz-spaces-python/blob/master/requirements.txt

I'm guessing the issue with fiona not being on the list, but being required is that it is a dependency of one of the listed dependencies, so it gets picked up. Just a guess though

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/xyzspaces) and found some lint.

Here's what I've got...

For recipes/xyzspaces:

  • requirements: host: turfpy>=0.0.3 must contain a space between the name and the pin, i.e. turfpy >=0.0.3

@conda-forge-linter
Copy link

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 (recipes/xyzspaces) and found it was in an excellent condition.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/xyzspaces) and found some lint.

Here's what I've got...

For recipes/xyzspaces:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@conda-forge-linter
Copy link

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 (recipes/xyzspaces) and found it was in an excellent condition.

@deeplook
Copy link
Contributor Author

deeplook commented Aug 29, 2020

I don't seem to put the pip-only dependencies like "turfpy" and "geobuf" at the right place. When trying to run conda smithy recipe-lint . locally I get "No azure token. Create a token and put it in ~/.conda-smithy/azure.token. Running out of ideas...

Maybe this is still relevant, which seems to suggest I cannot list pip-only dependencies in a recipe? conda/conda-build#548 and conda-forge/conda-forge.github.io#28.

@synapticarbors
Copy link
Member

@deeplook - we don't really want any pip-only dependencies. We'll need to open separate pull requests for turfpy and geobuf and get those merged before we can move forward with this recipe.

@deeplook
Copy link
Contributor Author

deeplook commented Aug 31, 2020

@synapticarbors While I can imagine the reasoning (stability, etc.) behind not wanting to support plain pip packages (which is somewhat ironic, as pip is still needed, I believe), I have failed to see a bold argument like this made on https://conda-forge.org/docs/index.html. I think this could help people spend more time on relevant things (like convincing other maintainers to publish their packages on conda-forge). What do you think?

@synapticarbors
Copy link
Member

@deeplook -- I've submitted a pull request (#12514) to address the missing dependencies.

@deeplook
Copy link
Contributor Author

@synapticarbors
Copy link
Member

@deeplook -- I restarted the build in this PR and it looks like linux is now passing. Just waiting on the others.

@synapticarbors
Copy link
Member

@deeplook -- can you confirm that the other co-maintainers have agreed to be listed? Otherwise they should comment here saying they are.

@deeplook
Copy link
Contributor Author

@synapticarbors I confirm.

@deeplook
Copy link
Contributor Author

@synapticarbors Anything else I need to do? Otherwise this is ready for review.

@synapticarbors synapticarbors merged commit 3e66af2 into conda-forge:master Aug 31, 2020
@deeplook
Copy link
Contributor Author

Thanks!

@deeplook
Copy link
Contributor Author

@synapticarbors BTW, the description field on https://anaconda.org/conda-forge/xyzspaces is empty despite the description given in the meta.yaml file. Am I missing something?

@synapticarbors
Copy link
Member

No packages on conda-forge have that populated (e.g https://anaconda.org/conda-forge/numpy). The description shows up in https://github.com/conda-forge/xyzspaces-feedstock#about-xyzspaces

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

Successfully merging this pull request may close these issues.

3 participants