Fix: ncss netcdf temp file permission error on Windows

[MoonRaker]

These changes were made in response to the inability of python to open a temporary file twice on Windows systems, Permission Denied error, thus preventing ncss.get_data() queries in Siphon. Issue #24 NetCDF tempfile handling broken on windows.

The temporary file is now persistent but is then deleted at exit. The temporary file will only be removed if .close() method is called on the netCDF Dataset. With this fix the error message disappears and all nosetests pass. It has not been tested on Linux/Unix.

Siphon v0.3.1
Code that reproduces the error (also fails 2 nosetests; test_netcdf_point, test_netcdf4_point):
best_gfs = TDSCatalog('http://thredds.ucar.edu/thredds/catalog/grib/NCEP/GFS/Global_0p5deg/catalog.xml?dataset=grib/NCEP/GFS/Global_0p5deg/Best')
best_gfs.datasets
best_ds = list(best_gfs.datasets.values())[0]
best_ds.access_urls
ncss = NCSS(best_ds.access_urls['NetcdfSubset'])
query = ncss.query()
query.lonlat_point(-110.9, 32.2).vertical_level(100000).time_range(start, end)
query.variables('Temperature_isobaric').accept('netcdf')
data = ncss.get_data(query)

...line 351, in read_netcdf
return Dataset(tmp_file.name, 'r')
File "netCDF4_netCDF4.pyx", line 1616, in netCDF4._netCDF4.Dataset.init (netCDF4_netCDF4.c:9989)
RuntimeError: Permission denied

[dopplershift]

For cleanliness, can you move atexit import to the first line, and the from os to just under from io?

[MoonRaker]

Absolutely. What is the reasoning? Just for my future reference?

[dopplershift]

We group the imports (based partially on PEP8) by standard library (first full import, then partial import), then third party, then our own; arranged alphabetically. Doing it consistently makes it easy to find stuff when you have quite a few imports.

[dopplershift]

Awesome, thanks for the contribution. Mostly just a bunch of little (picky) stuff. This looks identical to what I had originally, before I "discovered" the cleaner approach on *nix.

What I wonder is whether we should just enable this method on windows and stick with the original method on *nix. @lesserwhirls any opinions here?

[MoonRaker]

Thanks for the comments, I appreciate the input for sure. Either way is fine with me.

[landscape-bot]

Repository health decreased by 0.26% when pulling 556e456 on MoonRaker:master into 6f05e79 on Unidata:master.

• 1 new problem was found (including 1 error and 0 code smells).
• No problems were fixed.

[dopplershift]

I think I like the approach described at http://stackoverflow.com/a/15235559 (and mentioned in #24) better. Any thoughts? I'm willing to take a pass at implementing that approach, though it might be a week before I can do so; feel free to try it yourself if you're so inclined.

Either way, if you could test it and see if it actually works on Windows, that would be very helpful.

[MoonRaker]

It seems like a cleaner approach. I'll give it a go. At first pass though the entire implementation of the temporaryfile might have to look different otherwise it will require modifying the netCDF library since that is where the second open operation takes place.

[dopplershift]

paperbag moment

You're right, I didn't think things through. We can go with your original approach, though I'd like to leave the original code for OSX/Linux and only use the atexit approach on Windows.

[MoonRaker]

I have updated so that the usage for OSX/Linux is the same as before and only the Windows temporary file is handled with atexit.

[landscape-bot]

Repository health decreased by 0.27% when pulling 299ae19 on MoonRaker:master into 6f05e79 on Unidata:master.

• 1 new problem was found (including 1 error and 0 code smells).
• No problems were fixed.

[dopplershift]

Overall, I think we're almost there. There are some pep8 issues picked up by pylint:

siphon/ncss.py:347:9: N806 variable in function should be lowercase
siphon/ncss.py:352:47: E231 missing whitespace after ','
siphon/ncss.py:365:1: E302 expected 2 blank lines, found 1

Also need to address the remaining inline comments. The actual test failures are caused by a new version of vcrpy. I'll take care of fixing that, but that will mean you'll need to rebase on top of the latest master once I merge.

[dopplershift]

PermissionError it turns out is >= Python 3.3. I don't mind dropping Python 3.2, but I'm not quite prepared to drop Python 2.7. Since PermissionError is a subclass of OSError, can you use that instead?

[MoonRaker]

That should be fine. I have also taken care of the inline pep8 issues.

[landscape-bot]

Code quality remained the same when pulling ba21c67 on MoonRaker:master into a0ebefb on Unidata:master.

[landscape-bot]

Code quality remained the same when pulling ba21c67 on MoonRaker:master into a0ebefb on Unidata:master.

[dopplershift]

This looks good. Thanks for going around with me on it.

[MoonRaker]

I appreciate your patience and helping me work through it.