issue_comments

@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...

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

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).

@brhillman - A PR to fix the iteritems issue can happen now. You could also add a test to the allowed_failures section: https://github.com/pydata/xarray/blob/c748062ef4353f5e249553b06a198a44c71083c4/.travis.yml#L42-L61

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.

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

Yep, just saw that in https://github.com/conda-forge/pynio-feedstock/pull/30

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

@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.

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.

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.

@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.

@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.