Enable user-specified output directories in Proj #105

spencerkclark · 2016-10-28T19:48:19Z

This PR provides the quick fix discussed in #103. It also adds local test data files, which add some basic test coverage to Calc and also verify that output files are saved to the right places.

spencerkclark · 2016-10-28T23:23:54Z

@spencerahill I was a bit clumsy getting the tests configured properly (it took me a while to realize how to set up the paths to the test data on Travis), but I think I've got things working. Let me know if you're on board with how I made the changes in Calc, and if you're good with the test structure. Thanks!

spencerkclark · 2016-10-29T11:37:15Z

@spencerahill just so you know, I just rebased / squashed the commits offline to clean up the commit message

spencerahill · 2016-10-30T02:49:41Z

aospy/test/test_basic_calc.py

+
+class TestBasicCalc(unittest.TestCase):
+    def setUp(self):
+        print os.getcwd()


Did you want to keep this in? If so, include from __future__ import print_function and then use print(os.getcwd()) for Python 3 compatibility.

spencerahill · 2016-10-30T03:07:03Z

Strange, something with my local setup is causing errors when I try to run the new test (even though they're working on Travis): https://gist.github.com/spencerahill/90e751781812266f21dfdd01d3a4a426

I have seen that fp error from scipy before, but can't recall precisely what causes it. I have confirmed that the test netcdf files are in place and not corrupted.

I'll have to come back to this.

spencerkclark · 2016-10-30T11:37:21Z

@spencerahill How did you go about setting up your environment? One thing that stands out immediately is that it seems it doesn't have dask installed, which is required for a call that gets made to xr.open_mfdataset in model.py. netCDF4 is also a requirement to run the tests now (it wasn't in the past); it is needed for decoding CF attributes. Check out the new ci/environement-py27.yml, for the comprehensive list.

Addressing the import errors may fix everything (the failed tests are simply due to the fact that files are not produced, because of errors upstream -- the import errors could account for all of that since all the calculations start by reading in grid files).

That said, you seem to be using python 3.5 and until now we didn't have Travis set up to test in that environment. I added that to our set up, but as you can see, there are issues with imports of the test aospy object libraries within the test modules that prevent the tests even from being fully collected (for python 3.5 but not python 2.7). I'm not sure what's causing that (in your local environment that doesn't seem to have been a problem).

spencerkclark · 2016-10-30T12:08:26Z

@spencerahill Things seem to be working as we'd expect now for python 3.5; all I needed to do was switch from implicit relative imports to explicit ones in test_objs/. E.g. go from this:

import models

to this:

from . import models

The only thing is that there are now some "unexpected success" tests in the test_aospy module associated with division. How should we handle those?

spencerahill · 2016-10-31T19:33:18Z

unexpected success" tests in the test_aospy module associated with division.

Let's just remove the decorator. Probably at some point I screwed up the division logic, saw the tests were failing and slapped it on, and then forgot to change the tests whenever it was fixed again!

spencerahill · 2016-10-31T19:42:43Z

netCDF4 is also a requirement to run the tests now (it wasn't in the past); it is needed for decoding CF attributes.

Yes, sorry, this was my rookie mistake...borrowing my wife's computer and apparently forgot to install netCDF4.

I'm now getting 100% passing tests on 2.7 and 3.5 with the aforementioned expected failures fix. That resolved, will finally turn to the actual substance of what you've done!

spencerahill · 2016-10-31T19:47:25Z

aospy/calc.py

+            self._save_files(data, dtype_out_time)
+        if save_tar_files:
+            self._save_tar_files(dtype_out_time)
+        logging.info('\t{}'.format(self.path_out[dtype_out_time]))

    def _load_from_scratch(self, dtype_out_time, dtype_out_vert=False,


Let's rename this too... _load_from_disk? All "scratch" references should be gone.

spencerahill · 2016-10-31T19:49:18Z

aospy/calc.py

@@ -897,7 +897,7 @@ def _load_from_scratch(self, dtype_out_time, dtype_out_vert=False,

    def _load_from_archive(self, dtype_out_time, dtype_out_vert=False):


Let's rename _load_from_tar. Is any of this logic unique to /archive other than the dmget call? For the time being, add a try/except guard around the dmget call here as well, in case anybody tries using this from outside GFDL.

spencerkclark · 2016-10-31T20:07:22Z

Probably at some point I screwed up the division logic, saw the tests were failing and slapped it on, and then forgot to change the tests whenever it was fixed again!

There must be a python version dependence here though, since it fails in python 2.7, but not python 3.5? Maybe just skip them and fix things in a later PR?

spencerahill · 2016-10-31T20:56:59Z

There must be a python version dependence here though, since it fails in python 2.7, but not python 3.5? Maybe just skip them and fix things in a later PR?

Sorry, yet more rookie environment mistakes! I didn't have pytest installed for 2.7, and didn't realize that even though I had source activated my py27 env, it was still using the 3.5 pytest. You're right, they fail on 2.7. This is probably straightforward but not important right now (xref #50)...just do whatever you think is best.

One thing to keep in mind is that separate runs.py, models.py, etc. modules aren't strictly necessary: it could all be a single module, which may make more sense for small snippet-like projects such as the test one.

spencerahill · 2016-10-31T21:00:00Z

Final comment, then let's merge: directory naming; let's rename local_test_objs to data, with subdirectories netcdf and objects. (Just trying to emulate xarray's structure)

Another nice piece of work @spencerkclark, despite my best efforts to confuse you! Thanks, as always I really appreciate it.

spencerkclark · 2016-10-31T21:54:10Z

One thing to keep in mind is that separate runs.py, models.py, etc. modules aren't strictly necessary: it could all be a single module, which may make more sense for small snippet-like projects such as the test one.

Final comment, then let's merge: directory naming; let's rename local_test_objs to data, with subdirectories netcdf and objects. (Just trying to emulate xarray's structure)

Good thoughts, I have modified things accordingly. Perhaps the edits I needed to make to the path names to allow for this directory structure are not the most idiomatic, but they get the job done.

@spencerahill, if you're cool with those, I think we're all set!

TST Add simple tests and verify that results files are produced TST Add tests for reg.av and reg.ts calcs TST Remove unnecessary print statement TST Add python 3.5 test environment TST Switch to explicit relative imports in test_objs Remove all references to scratch and archive in method names TST Skip version dependent constant division operator tests TST Refactor test objs into single module and rename directories

spencerahill · 2016-11-01T15:23:21Z

Awesome. Sorry, one last nit: let's make the example object names more explicit: example_proj, example_model, example_run. im is a little cryptic. Sorry I didn't catch that before.

spencerahill · 2016-11-01T20:40:47Z

Awesome, thanks @spencerkclark!

spencerkclark force-pushed the direc-out2 branch from eedd44d to 1113015 Compare October 29, 2016 11:34

spencerkclark force-pushed the direc-out2 branch from 8dc72e0 to faa273f Compare October 29, 2016 12:13

spencerahill reviewed Oct 30, 2016

View reviewed changes

spencerahill reviewed Oct 31, 2016

View reviewed changes

spencerkclark force-pushed the direc-out2 branch from 3cbb864 to 3ce1802 Compare November 1, 2016 11:42

TST Give test objects more generic names

c27712c

spencerahill merged commit d309506 into spencerahill:develop Nov 1, 2016

spencerkclark deleted the direc-out2 branch November 2, 2016 01:13

This was referenced Nov 21, 2016

Add testing data to aospy #62

Closed

Get continuous integration tool(s) properly configured #63

Closed

spencerahill mentioned this pull request Dec 2, 2016

Proj.direc_out is unused #103

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable user-specified output directories in Proj #105

Enable user-specified output directories in Proj #105

spencerkclark commented Oct 28, 2016 •

edited

Loading

spencerkclark commented Oct 28, 2016 •

edited

Loading

spencerkclark commented Oct 29, 2016

spencerahill Oct 30, 2016

spencerahill commented Oct 30, 2016

spencerkclark commented Oct 30, 2016

spencerkclark commented Oct 30, 2016 •

edited

Loading

spencerahill commented Oct 31, 2016

spencerahill commented Oct 31, 2016

spencerahill Oct 31, 2016

spencerahill Oct 31, 2016

spencerkclark commented Oct 31, 2016 •

edited

Loading

spencerahill commented Oct 31, 2016

spencerahill commented Oct 31, 2016

spencerkclark commented Oct 31, 2016

spencerahill commented Nov 1, 2016

spencerahill commented Nov 1, 2016

		@@ -897,7 +897,7 @@ def _load_from_scratch(self, dtype_out_time, dtype_out_vert=False,

		def _load_from_archive(self, dtype_out_time, dtype_out_vert=False):

Enable user-specified output directories in Proj #105

Enable user-specified output directories in Proj #105

Conversation

spencerkclark commented Oct 28, 2016 • edited Loading

spencerkclark commented Oct 28, 2016 • edited Loading

spencerkclark commented Oct 29, 2016

spencerahill Oct 30, 2016

Choose a reason for hiding this comment

spencerahill commented Oct 30, 2016

spencerkclark commented Oct 30, 2016

spencerkclark commented Oct 30, 2016 • edited Loading

spencerahill commented Oct 31, 2016

spencerahill commented Oct 31, 2016

spencerahill Oct 31, 2016

Choose a reason for hiding this comment

spencerahill Oct 31, 2016

Choose a reason for hiding this comment

spencerkclark commented Oct 31, 2016 • edited Loading

spencerahill commented Oct 31, 2016

spencerahill commented Oct 31, 2016

spencerkclark commented Oct 31, 2016

spencerahill commented Nov 1, 2016

spencerahill commented Nov 1, 2016

spencerkclark commented Oct 28, 2016 •

edited

Loading

spencerkclark commented Oct 28, 2016 •

edited

Loading

spencerkclark commented Oct 30, 2016 •

edited

Loading

spencerkclark commented Oct 31, 2016 •

edited

Loading