pynio backend broken in python 3

[brhillman]

pynio recently released a development python 3 port (see NCAR/pynio#10). The pynio backend in xarray needs a slight fix to use this though; the get_variables() method in the NioDataStore class tries to access the iteritems() method of the variables object, which no longer exists in python3. The simple fix is to replace iteritems() with items() in this method. I've made this fix on a local version of the code, and it seems to work as expected. I would be happy to put together a PR if nobody has done such, and if someone could point me to any documentation or recommendations on the preferred conventions for structuring PRs for the project (I have not contributed before). Thanks!

[jhamman]

@brhillman - that would be great. I don't think anyone has worked on this yet and we'd be happy to have you contribute here.

We have some contributor guidelines in the works (see #1485).

Generally, we'll want to add the pynio package to one or more of the python3 test suites (here would be a good place to start).

Put your changes on a branch (e.g. feature/python3_pynio), run the test suite (py.test), follow pep8 formatting, and then push to your fork and open a PR. The template checklist in the PRs helps us make sure we're catching the main things.

[brhillman]

@jhamman okay I'll take a look and put this together. Would you prefer a single PR to both fix the issue and add a pynio test, or separate PRs for those two changes? I've got a bit of reading to do to get up to speed on adding a test I think.

[jhamman]

One PR is fine. I don't think you'll need to add any tests, just add pynio to the conda environment file I linked to above and the existing test base should run for python 3 with pynio.

We may need to ping @ocefpaf to help us get a conda-forge build of pynio for python3 repo.

[ocefpaf]

Do you know if the py3k support made into pynio 1.5.0 release? If so I can "fix" the conda package easily, if not I am unsure what source I should use to enable the Python 3 builds.

[brhillman]

@ocefpaf no, I don't think so. I think the only version of pynio that has py3 support is the dev snapshot on the ncar conda-forge channel. This is the version I used to get pynio to work in my python 3 environment so that I could use xarray to read ERA-Interim grib files. I installed the dev snapshot via conda install -c ncar -c conda-forge pynio=dev python=3.

[ocefpaf]

Yep, just saw that in conda-forge/pynio-feedstock#30

So we need to wait... Hopefully they will release it soon. Thanks!

[brhillman]

@ocefpaf @jhamman should I wait to put together the PR then until they release an official version of pynio with py3 support?

[jhamman]

@brhillman - A PR to fix the iteritems issue can happen now. You could also add a test to the allowed_failures section:

xarray/.travis.yml

Lines 42 to 61 in c748062

	allow_failures:
	- python: 3.6
	env:
	- CONDA_ENV=py36
	- EXTRA_FLAGS="--run-flaky --run-network-tests"
	- python: 3.6
	env: CONDA_ENV=py36-netcdf4-dev
	addons:
	apt_packages:
	- libhdf5-serial-dev
	- netcdf-bin
	- libnetcdf-dev
	- python: 3.6
	env: CONDA_ENV=py36-dask-dev
	- python: 3.6
	env: CONDA_ENV=py36-pandas-dev
	- python: 3.6
	env: CONDA_ENV=py36-bottleneck-dev
	- python: 3.6
	env: CONDA_ENV=py36-condaforge-rc

that pulls the ncar dev build down. That would allow us to work out some of the kinks while they get their final python3 release ready.

More broadly, if we don't have a viable way to install the package, perhaps your effort would be better spent helping PyNio get Python3 into a full release.

[brhillman]

Okay @jhamman excuse my ignorance on the test suite, I've never worked with pytest or Travis CI before, but if I understand correctly, this would involve adding a section like:

- python: 3.6
   env: CONDA_ENV=py36-pynio-dev

to xarray/.travis.yml, and then add a corresponding ci/requirements-py36-pynio-dev.yml file to list the dependencies for that test environment? I've gotten the tests to run and pass on my environment just by adding an entry to ci/requirements-py36.yml that reads
- pynio=dev python=3 and adding an entry that reads - ncar to the "channels" section, but maybe this is the wrong place to add this?

FYI I also had to make a change to xarray/tests/__init__.py to import pynio correctly (the test would have never run, regardless of whether or not pynio because it tried to import the wrong package name).

[jhamman]

Yep, it sounds like you have it about right. Why don't you open a PR and we can iterate further there.

[brhillman]

@jhamman okay I figured it out, added the tests, confirmed that the Nio tests get run, and they all pass (other than a dask test, but I think that's expected right now?). But, I'm getting some strange results from git diff upstream/master | flake8 --diff, on lines that I did not modify, and some lines that I copied the syntax over from existing files (i.e., ci/requirements-py36-pynio-dev.yml, where it complains about indentation). Is this a concern? PR ready to go other than that. My branch is here: https://github.com/brhillman/xarray/tree/fix-py3-pynio-backend.

edit: oops, commented before I got your response. Opening PR now and we can iterate there...