Skip to content

Add eccodes python interface#9731

Closed
StephanSiemen wants to merge 9 commits intoconda-forge:masterfrom
StephanSiemen:add-eccodes-python-interface
Closed

Add eccodes python interface#9731
StephanSiemen wants to merge 9 commits intoconda-forge:masterfrom
StephanSiemen:add-eccodes-python-interface

Conversation

@StephanSiemen
Copy link
Contributor

This will package the Python 2 and 3 interfaces to eccodes based on CFII from PyPi

Hi @conda-forge/help-python, we finally manage to package the proper Python interface to eccodes here. This takes the version independent implementation from PyPi. As soon this package is accepted, we can add this as dependency to 'eccodes' and can retire 'python-eccodes' for good :-)

Thanks for your help!

Checklist

  • [x ] License file is packaged (see here for an example)
  • [x ] Source is from official source
  • [ x] Package does not vend other packages
  • [x ] Build number is 0
  • [x ] A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details)
  • [x ] GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there

This will package the Python 2 and 3 interfaces to eccodes based on CFII from PyPi
@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/eccodes-python) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/eccodes-python:

  • license_file entry is missing, but is expected.

@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/eccodes-python) and found it was in an excellent condition.

@chrisburr
Copy link
Member

Is this vending gribapi?

Copy link
Member

@chrisburr chrisburr left a comment

Choose a reason for hiding this comment

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

Assuming eccodes is the authoritative source of gribapi, I think these are the changes that are needed.

@StephanSiemen
Copy link
Contributor Author

Is this vending gribapi?

'eccodes' supersedes 'grib_api'. The library handles now also BUFR and not only GRIB data, therefore it was renamed. grib_api is now frozen. This interface would actually work with grib_api, but all BUFR related commands would fail.

StephanSiemen and others added 3 commits October 3, 2019 20:56
Co-Authored-By: Chris Burr <chrisburr@users.noreply.github.com>
Co-Authored-By: Chris Burr <chrisburr@users.noreply.github.com>
@StephanSiemen
Copy link
Contributor Author

@chrisburr the pr is blocked because of an outstanding change request by you. I have difficulties to see what it refers to. Any help is appreciated. Thanks

@chrisburr
Copy link
Member

That's just the way GitHub shows that I had previously requested changes. It doesn't block this from being merged by anyone else on the staged-recipes team, just encourages them to check the comments to see if it's been fixed. (In this case it was the licence)

One last question, would it be better to update https://github.com/conda-forge/python-eccodes-feedstock/ instead of making a new feedstock?

cc @conda-forge/python-eccodes

@StephanSiemen
Copy link
Contributor Author

That's just the way GitHub shows that I had previously requested changes. It doesn't block this from being merged by anyone else on the staged-recipes team, just encourages them to check the comments to see if it's been fixed. (In this case it was the licence)

One last question, would it be better to update https://github.com/conda-forge/python-eccodes-feedstock/ instead of making a new feedstock?

cc @conda-forge/python-eccodes

Thanks @chrisburr for your help. 'python-eccodes' as is currently setup is the building of eccodes library and the python interface through the old swig setup which only works for Python2. Users asked us not to touch this setup to avoid changes to their dependencies (we anyway have no write access) and instead have a new clean setup where the library and python interface are separated. We were suggested to use the same name as on PyPi to reduce confusion. At SciPy I was told 'python-eccodes' as an Python2-only package would be frozen at the end of year and then this would be the one everyone would use in future. I agree, with hindsight this is not the ideal road to take ...

@chrisburr this is really not a bad question to ask, and if anyone from @conda-forge/python-eccodes has a better suggestion, we are very happy to listen. Now is the time ;-)

@kmuehlbauer
Copy link
Contributor

As soon this package is accepted, we can add this as dependency to 'eccodes' and can retire 'python-eccodes' for good :-)

@StephanSiemen I would agree with the latter but I do not understand the first. How should this new package eccodes-python be added to eccodes?

@kmuehlbauer
Copy link
Contributor

@chrisburr The thing was talked about in conda-forge/python-eccodes-feedstock#49 and the new bindings added in conda-forge/python-eccodes-feedstock#48 with efforts from @beckermr.

For now we can conda install the new eccodes-python (from https://github.com/ecmwf/eccodes-python) as python-eccodes. The naming scheme is still an issue and maybe it was not a good idea to name it python-eccodes back then (#742 and #750) (my bad, I choose it having python-ecmwf_grib as guideline).

Now I'm not really sure, if I would still argue like in my comment here.

Maybe we could go the gdal way, having multiple outputs from one feedstock. @ocefpaf what does CF core suggest in those cases.

@ocefpaf
Copy link
Member

ocefpaf commented Oct 4, 2019

Maybe we could go the gdal way, having multiple outputs from one feedstock. @ocefpaf what does CF core suggest in those cases.

This is probably better. I cannot help at the moment (day job travel and some deadlines until the end of the year). However, if someone wants to tackle this feel free to ping me for a review. The gdal and matplotlib feedstocks are good examples on how to do this.

home: https://github.com/ecmwf/eccodes-python
license: Apache-2.0
license_family: Apache
license_file: LICENSE.txt
Copy link
Contributor

@kynan kynan Oct 6, 2019

Choose a reason for hiding this comment

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

Note that this file name is relative to the package root, not relative to the recipe. It is therefore not necessary to include the license with the recipe. This should be LICENSE to match the file name in the source dist.

@stale
Copy link

stale bot commented Mar 4, 2020

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on master so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

@stale stale bot added the stale will be closed in 30 days label Mar 4, 2020
@stale
Copy link

stale bot commented Apr 4, 2020

Hi again! About a month ago, we commented on this PR saying it would be closed in another month if it was still inactive. It has been a month and so now it is being closed. Thank you so much for making it in the first place and contributing to the community project that is conda-forge. If you'd like to reopen this PR, please feel free to do so at any time!

Cheers and have a great day!

@stale stale bot closed this Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale will be closed in 30 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants