Skip to content
This repository has been archived by the owner on Oct 11, 2019. It is now read-only.

Add recipes for OpenClimateGIS #590

Merged
merged 10 commits into from
Nov 16, 2015
Merged

Add recipes for OpenClimateGIS #590

merged 10 commits into from
Nov 16, 2015

Conversation

bekozi
Copy link
Contributor

@bekozi bekozi commented Nov 13, 2015

This PR adds recipes for OpenClimateGIS and dependencies not covered by IOOS.

  • Building the "next" branch of OpenClimateGIS is recommended for netCDF4 and OSX compatibility.
  • Python 2.7 only.
  • The default conda Fiona package conflicts with GDAL 2.0. The Fiona release supporting 2.0 is in prep. In the meantime, the trunk is built.
  • OpenClimateGIS uses "cfunits" v. "cf_units"...
  • Don't have many .bat files.

The build script looks something like below. There are nested environment variables in the meta.yaml files. Is it better for your system to use embedded versions/tags?

export CBUILD_OCGIS_TAG=next
export CBUILD_OCGIS_VERSION=1.2.0.dev1
export CBUILD_FIONA_TAG=master
export CBUILD_FIONA_VERSION=1.6.2.dev1
export CBUILD_CFUNITS_TAG=v1.1
export CBUILD_CFUNITS_VERSION=1.1

conda build ocgis
# Optional dependency.
conda build cfunits
# Install with optional dependencies (no ESMF or ICCLIM yet).
conda install ocgis cfunits rtree

I tested building on a "continuum/anaconda" Docker image. I noticed you are using a CentOS6 image for your builds. We have used CentOS6 as a build image before without issue. Anyway, let's see how it goes!

@ocefpaf
Copy link
Member

ocefpaf commented Nov 13, 2015

@bekozi Thanks for the PR!

I will review it shortly.

@ocefpaf
Copy link
Member

ocefpaf commented Nov 13, 2015

OpenClimateGIS uses "cfunits" v. "cf_units"...

Any chance I can convince you to switch? The libraries should be pretty similar, but cf_units has a better community support.

@ocefpaf
Copy link
Member

ocefpaf commented Nov 13, 2015

Don't have many .bat files.

Please add the syntax:

build:
    skip: True  # [win]

You may also skip Python 3 by adding # [win and py3k]

#!/usr/bin/env bash

$PYTHON setup.py install --record record.txt
$PYTHON test/run_tests.py
Copy link
Member

Choose a reason for hiding this comment

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

Do you think you can add this test to the recipe directory? That way conda runs it automatically without this call.
(And we can explicitly see what is being tested.)

@rsignell-usgs
Copy link
Member

@bekozi, do you feel behind the 🎱 ? 😸

- cligj
- munch
- click-plugins
- libpq # [osx]
Copy link
Member

Choose a reason for hiding this comment

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

Should cfunits be here?

@ocefpaf
Copy link
Member

ocefpaf commented Nov 13, 2015

export CBUILD_OCGIS_TAG=next
export CBUILD_OCGIS_VERSION=1.2.0.dev1
export CBUILD_FIONA_TAG=master
export CBUILD_FIONA_VERSION=1.6.2.dev1
export CBUILD_CFUNITS_TAG=v1.1
export CBUILD_CFUNITS_VERSION=1.1

@bekozi We use Travis-CI, CircleCI, and AppVeyor to automatically build the recipes for OSX, Linux, and Windows respectively. Everything is tied together with obvious-ci. Note that is obvious-ci who create the build matrix and takes care of the dependencies build order.

That said, it is better to avoid the environment variable and put them inside the meta.yaml files.

@bekozi
Copy link
Contributor Author

bekozi commented Nov 13, 2015

@rsignell-usgs: Ha! No one said it would be easy. 😃

@ocefpaf: I addressed the comments. Anything in question or requiring input is addressed below. Thanks for the thorough review!

#590 (comment)
Any chance I can convince you to switch? The libraries should be pretty similar, but cf_units has a better community support.

Yeah probably. I'll look into it. I'm guessing it's okay to keep pressing forward for now. You saw the ticket.

#590 (comment)
Why do you call postgresql libpq? Will the default channel version suffice?

An oversight. That is all. 😃

#590 (comment)
If so consider pinning gdal to <2.0.0 in ocgis instead. We avoid as much as we can to use master/dev versions.

We can go this route. There are complications with osx and GDAL versions < 2.0. See ContinuumIO/anaconda-issues#380 (comment) for reference. For Linux there are no issues. I understand the desire to avoid development versions. Maybe we can get this working, and then I can publish a minor release we can build against (before merging)?

I am not sure when the fiona version supporting 2.0 will be out. They are working on it! That dev can then go away too.

Are you interested in supporting a GDAL build?

#590 (comment)
You might want to pin gdal version here.

That should be taken care of in the fiona recipe?

#590 (comment)
Should cfunits be here?

It's technically an optional dependency. However, I don't really see why rtree and cfunits-python should not be added as the impact is small. I had them on the original recipe.

I'll keep an eye on the CI builds too...

Removed support for Python 3. This could be an easy fix, but it is
removed for now.
@rsignell-usgs
Copy link
Member

@bekozi , thanks for plugging away at this! We'll get there!!!

@bekozi
Copy link
Contributor Author

bekozi commented Nov 13, 2015

The cfunits-python build is failing for Python 3, yet building Python 3 is supposedly disabled. What am I missing?

py3k skipped: NESII@921087e#diff-04a77a7db3f9089e7c58082e33011a0cR11

CI Failure:

==================================== ERRORS ====================================
_____________________ ERROR collecting test/test_Units.py ______________________
../../envs/_test/lib/python3.5/site-packages/_pytest/python.py:584: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
../../envs/_test/lib/python3.5/site-packages/py/_path/local.py:650: in pyimport
    __import__(modname)
E     File "/opt/conda/conda-bld/work/test/test_Units.py", line 138
E       print 'cfunits-python version:', cf.__version__
E                                     ^
E   SyntaxError: Missing parentheses in call to 'print'
=========================== 1 error in 0.10 seconds ============================

TESTS FAILED: cfunits-python-1.1-py35_0 ./scripts/run_docker_build.sh returned exit code 1

@ocefpaf
Copy link
Member

ocefpaf commented Nov 13, 2015

The cfunits-python build is failing for Python 3, yet building Python 3 is supposedly disabled. What am I missing?

I forgot to mention that you have to add python >=2.7,<3 in the build section.

requirements:
build:
- python
- udunits2
Copy link
Member

Choose a reason for hiding this comment

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

Question: cfunits-python does not use setuptools?

Added Python version restrictions for packages not supporting Python 3.
@ocefpaf
Copy link
Member

ocefpaf commented Nov 15, 2015

@bekozi we are almost there. The OSX failure (Travis-CI) is because cfunits is failing to find the udunits2 shared library.

Finding shared libraries in conda and load them with ctypes is a known issue. The solutions are to use a setup.cfg, like we do for cf_units, or to patch the Software to forcing it to point to the conda version, like we do for shapely, rtree, and many others here.

@@ -0,0 +1,3 @@
#!/usr/bin/env bash

$PYTHON setup.py install --record record.txt
Copy link
Member

Choose a reason for hiding this comment

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

Please add the --single-version-externally-managed too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cfunits-python does not use setuptools. I sent them a PR for it some time ago, but they want to stick with distutils.

Copy link
Member

Choose a reason for hiding this comment

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

No problem. But py3k, setuptools, and easy udunits support via configparse seems like good reasons to switch to cf_units 😉

Copy link
Member

Choose a reason for hiding this comment

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

+1 for switching to cf_units. We went through a lot of pain with the others, and cf_units has the brightest future, for sure, now that the British Met Office is on board.

- Removed Python version restrictions in requirements.
- Removed support for OSX until new OCGIS release.
- Removed packages not required for latest OCGIS release and OSX
  compatibility.
Needed two lines to block win/osx and py3k.
Build was allowing OSX and Python 2.x. All OSX builds are now skipped.
GDAL > 2.0 was still being installed. Added constraints on "gdal" and
 "libgdal". Also forget to restrict the "netcdf4" version.
Not sure how these popped back into version control, but they have
been removed. Fiona was causing the builds to fail for some platforms.
The selectors were allowing for win and py2k.
@bekozi
Copy link
Contributor Author

bekozi commented Nov 16, 2015

Linux builds are passing (finally). There is one failure on appveyor, but that is related to pyproj. I'm not sure if you want to merge this, or wait until I have tagged an ocgis release that will better support OSX (i.e. cf_units). The next release will also relax the netcdf4 version constraint and, depending on the timing, allow for GDAL 2.0. Let me know!

Regardless, thanks for the assistance and patience. This is a great build system!

@ocefpaf
Copy link
Member

ocefpaf commented Nov 16, 2015

Linux builds are passing (finally)

🎉

There is one failure on appveyor

Never mind AppVeyor. You are not building anything on Windows. I already sent a fix upstream. Pyproj is built against master 😱 and the failure is due to a new PR that introduced a bug. (See... That is why we don't build against master 😉)

I'm not sure if you want to merge this, or wait until I have tagged an ocgis release that will better support OSX

We can merge as is and people can start using ocgis on Linux. We just need to open an issue for the OSX support to keep track of what needs to be done.

Regardless, thanks for the assistance and patience.

Thank you for the perseverance!

This is a great build system!

Indeed. The credits shouldgo to @pelson who started all this for the scitools.

Looking forward to get the recipes for the ESMF working too.

ocefpaf added a commit that referenced this pull request Nov 16, 2015
Add recipes for OpenClimateGIS
@ocefpaf ocefpaf merged commit 0f3d555 into ioos:master Nov 16, 2015
@bekozi
Copy link
Contributor Author

bekozi commented Nov 17, 2015

Awesome. Thanks!

@bekozi bekozi deleted the ocgis-recipes branch November 17, 2015 15:20
- fiona
- gdal <2
- libgdal <2
- netcdf4 <=1.1.6
Copy link
Member

Choose a reason for hiding this comment

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

@bekozi I forgot. What is the reason behind a netCDF4-python <= 1.1.6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an odd issue with netCDF4-python v1.1.6+ that required this (very specific) workaround: https://github.com/NCPP/ocgis/blob/176a86e293e101877de1098a35f9efde73e83718/src/ocgis/interface/base/attributes.py#L32. I had trouble isolating the issue, but it seems like there is a rogue pointer somewhere in the netCDF4 attribute code. The workaround came after the 1.2 release, and it is fixed in the upcoming release. If this is causing a problem, I could add a patch file for the interim?

Copy link
Member

Choose a reason for hiding this comment

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

We can wait for a new release. I was just wondering why. Thanks for the explanation.

PS: This surfaced because I am re-building gdal 1.11.3 and fiona and I noticed that, enforcing netcdf4 <= 1.1.6 here, we end up with numpy < 1.10. That happens because there is not netCDF4-python 1.1.6 compiled against a newer numpy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Release should be next month. An IOOS-hosted gdal and fiona will be awesome!

There are already up: gdal, fiona, and rasterio. Note that the default channel split gdal into gdal+libgdal* while I left the whole package as just "gdal."

* The default channel gdal is actually only the python bindings, while libgdal is everything else (including libs). I prefer to avoid that confusion. A correct way would be to split in: gdal, python-gdal, and libgdal. But I am not sure that is worth the effort.

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

Successfully merging this pull request may close these issues.

None yet

3 participants