WIP: Use Pooch to download sample data #915

leouieda · 2018-08-10T17:23:04Z

Add it to metpy/cbook.py and include the dependency on setup.py. Create the registry file using pooch.make_registry and stored it in the metpy package.

Some tests fail because they use a glob to get all the files in the nids folder. Will try to replace that with a lookup of the Pooch registry.

I'm also not sure how to get it installed on the CIs. I added it to the setup.py. Is that enough?

Fixes fatiando/pooch#8

leouieda · 2018-08-10T20:43:24Z

The CircleCI build is complaining about netcdf4 but it passes locally. Any ideas?

jrleeman · 2018-08-17T17:37:34Z

CircleCI is an expected fail right now - we're trying to get this in today/over the weekend. Working on it this afternoon.

dopplershift

Thanks for contributing! This is a great start, just a few things I'd like to see.

dopplershift · 2018-08-17T20:37:48Z

MANIFEST.in

@@ -5,6 +5,7 @@ include LICENSE
 include CONTRIBUTING.md
 include versioneer.py
 include metpy/_version.py
+include metpy/registry.txt


How about something more descriptive--maybe static-data-manifest.txt?

Yeah, that would be better. @jrleeman should I fix this or are you gonna edit this branch?

Editing now 😄

dopplershift · 2018-08-17T20:39:39Z

environment.yml

@@ -35,3 +35,5 @@ dependencies:
  - cartopy
  - doc8
  - recommonmark
+  - pip:


Let's vendor in pooch temporarily while things settle just a bit and we work on things like conda packages. Without conda packages for pooch, we won't even be able to update MetPy's packages.

Yeah, I thought about that. I think I'll get started on the conda-forge package and we make new releases later with fixes required by metpy.

dopplershift · 2018-08-17T20:43:37Z

metpy/cbook.py


+def get_test_data(fname, as_file_obj=True):
+    """Access a file from MetPy's collection of test data."""
+    path = POOCH.fetch(fname)


Would be good to have a way to use our local git checkout of files directly so we can more easily bootstrap adding new files.

I thought of that while implementing this. The problem is that the directory is versioned. I got around it on the CIs by copying the data to the cache folder. That bypasses the download at least. A static link would be better locally to keep the folders in sync.

I think the way to do this is check "are we in a git repo?" and if so, then we'll set the environment variable to point the cache to the staticdata directory.

That wouldn't work because of the versioning. The env variable only points to the base level directory.

Just got a workaround going - verifying now.

jrleeman

So I think we're actually going to put some hooks in to look into the github directory, if it exists, first. We'll also do some checking to make sure those files do get into the registry.

leouieda · 2018-08-17T20:46:40Z

.travis.yml

@@ -108,8 +109,9 @@ install:
  - pip install ".[$EXTRA_INSTALLS]" --upgrade --upgrade-strategy=eager --no-index $PRE -f file://$PWD/$WHEELDIR $VERSIONS;

 script:
+  # Move the data to the cache location to avoid downloading it
+  - cp -r $TRAVIS_BUILD_DIR/staticdata/* $TEST_DATA_DIR/master


Turns out Travis doesn't like indented comments. This will keep it from downloading the data every time.

leouieda · 2018-08-17T21:07:06Z

So I think we're actually going to put some hooks in to look into the github directory, if it exists, first.

Wouldn't it be better to link staticdata to $TEST_DATA_DIR/master? No coding required.

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/__init__.py

@@ -0,0 +1,48 @@
+# pylint: disable=missing-docstring
+# Import functions/classes to make the API
+from . import version


F401 '.version' imported but unused

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/__init__.py

+# pylint: disable=missing-docstring
+# Import functions/classes to make the API
+from . import version
+from .core import Pooch, create


F401 '.core.create' imported but unused
F401 '.core.Pooch' imported but unused

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/__init__.py

+# Import functions/classes to make the API
+from . import version
+from .core import Pooch, create
+from .utils import os_cache, file_hash, make_registry


F401 '.utils.os_cache' imported but unused
F401 '.utils.file_hash' imported but unused
F401 '.utils.make_registry' imported but unused

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/_version.py

+        dirname = os.path.basename(root)
+        if dirname.startswith(parentdir_prefix):
+            return {
+                "version": dirname[len(parentdir_prefix) :],


E203 whitespace before ':'

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/_version.py

+    # starting in git-1.8.3, tags are listed as "tag: foo-1.0" instead of
+    # just "foo-1.0". If we see a "tag: " prefix, prefer those.
+    TAG = "tag: "
+    tags = set([r[len(TAG) :] for r in refs if r.startswith(TAG)])


E203 whitespace before ':'

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/_version.py

+    for ref in sorted(tags):
+        # sorting will prefer e.g. "2.0" over "2.0rc1"
+        if ref.startswith(tag_prefix):
+            r = ref[len(tag_prefix) :]


E203 whitespace before ':'

stickler-ci · 2018-08-17T22:17:10Z

metpy/_vendor/pooch/_version.py

+                tag_prefix,
+            )
+            return pieces
+        pieces["closest-tag"] = full_tag[len(tag_prefix) :]


E203 whitespace before ':'

stickler-ci · 2018-08-17T22:17:11Z

metpy/_vendor/pooch/tests/utils.py

+    """
+    registry = {
+        "tiny-data.txt": "baee0894dba14b12085eacb204284b97e362f4f3e5a5807693cc90ef415c1b2d",
+        "subdir/tiny-data.txt": "baee0894dba14b12085eacb204284b97e362f4f3e5a5807693cc90ef415c1b2d",


E501 line too long (99 > 95 characters)

stickler-ci · 2018-08-20T17:35:52Z

metpy/_vendor/__init__.py

+# Copyright (c) 2015 MetPy Developers.
+# Distributed under the terms of the BSD 3-Clause License.
+# SPDX-License-Identifier: BSD-3-Clause
+"""Vendor packages used in MetPy."""


W292 no newline at end of file

jrleeman · 2018-08-20T22:11:20Z

@dopplershift - any ideas on why codecov isn't ignoring this? Once things look better, I'll cleanup and rebase.

dopplershift · 2018-08-20T22:14:58Z

Looks like globs for codecov are not recursive. They give a solution in the linked docs.

leouieda · 2018-08-21T04:21:21Z

I started conda-forge/staged-recipes#6540 so once that is done we probably don't need the vendoring, right?

jrleeman · 2018-08-21T13:14:36Z

We'll probably vendor for this version of MetPy to make sure all the dust settles and then go to a conda forge install in 0.10.

leouieda · 2018-08-21T19:13:42Z

👍 just in case you want to try it out, the feedstock is ready https://github.com/conda-forge/pooch-feedstock

dopplershift · 2018-08-22T20:52:49Z

metpy/_vendor/pooch/_version.py

@@ -0,0 +1,557 @@
+# pylint: skip-file


Is it possible to nuke this file and version.py?

Probably. That's not used in the rest of the package. It is used in one of the tests (https://github.com/fatiando/pooch/blob/master/pooch/tests/test_integration.py#L13) but if you're not running those you could also nuke the entire tests folder.

dopplershift · 2018-08-22T20:55:31Z

metpy/_vendor/pooch/utils.py

+    'dev'
+
+    """
+    parse = Version(version)


I think this would work as:

return fallback if '+' in version else version

Yeah, using packaging is probably overkill here.

leouieda · 2018-08-22T22:05:59Z

What's the advantage of vendoring vs pinning the requirement to a given version? If the concern is pooch updating and breaking things, the pinning should solve that. Plus, nothing else relies on pooch so it wouldn't block anything else from updating (like numpy would).

That's an honest question, BTW. I had to look up "vendor" on wikipedia :)

dopplershift · 2018-08-22T22:15:59Z

You make an interesting point about pinning...

Add it to `metpy/cbook.py` and include the dependency on setup.py. Create the registry file using `pooch.make_registry` and stored it in the `metpy` package.

jrleeman · 2018-08-23T16:45:22Z

Okay, just did what should be the final history cleanup here - per review and CI this is ready to rock 🍾

leouieda · 2018-08-23T21:44:02Z

🎉 thanks!

leouieda requested review from dopplershift and jrleeman as code owners August 10, 2018 17:23

dopplershift added Area: Infrastructure Pertains to project infrastructure (e.g. CI, linting) Type: Feature New functionality labels Aug 17, 2018

dopplershift added this to the 0.9 milestone Aug 17, 2018

dopplershift added Area: DevTools Pertains to tools for developers and removed Area: Infrastructure Pertains to project infrastructure (e.g. CI, linting) labels Aug 17, 2018

jrleeman mentioned this pull request Aug 17, 2018

Data Cache #760

Closed

dopplershift reviewed Aug 17, 2018

View reviewed changes

jrleeman reviewed Aug 17, 2018

View reviewed changes

leouieda commented Aug 17, 2018

View reviewed changes

stickler-ci reviewed Aug 17, 2018

View reviewed changes

stickler-ci reviewed Aug 20, 2018

View reviewed changes

Unidata deleted a comment from codecov bot Aug 22, 2018

dopplershift reviewed Aug 22, 2018

View reviewed changes

leouieda and others added 2 commits August 23, 2018 10:44

Use Pooch to download sample data

b3fc756

Add it to `metpy/cbook.py` and include the dependency on setup.py. Create the registry file using `pooch.make_registry` and stored it in the `metpy` package.

Remove 32-bit windows builds

1b5d661

dopplershift approved these changes Aug 23, 2018

View reviewed changes

dopplershift merged commit 7ccd12e into Unidata:master Aug 23, 2018

dopplershift mentioned this pull request Aug 23, 2018

testdata missing #190

Closed

leouieda deleted the pooch branch August 23, 2018 21:44

jni mentioned this pull request Dec 17, 2018

Missing scientific (domain-specific) images and corresponding examples and tutorials scikit-image/scikit-image#3384

Open

WIP: Use Pooch to download sample data #915

WIP: Use Pooch to download sample data #915

Conversation

leouieda commented Aug 10, 2018 • edited

leouieda commented Aug 10, 2018

jrleeman commented Aug 17, 2018

dopplershift left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrleeman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leouieda commented Aug 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrleeman commented Aug 20, 2018

dopplershift commented Aug 20, 2018

leouieda commented Aug 21, 2018

jrleeman commented Aug 21, 2018

leouieda commented Aug 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leouieda commented Aug 22, 2018 • edited

dopplershift commented Aug 22, 2018

jrleeman commented Aug 23, 2018

leouieda commented Aug 23, 2018

leouieda commented Aug 10, 2018 •

edited

leouieda commented Aug 22, 2018 •

edited