add support for opening file from memory #652

thehesiod · 2017-04-28T04:10:35Z

fixes: #406 and #295, requires: Unidata/netcdf-c#394 to be fixed

thehesiod · 2017-04-28T04:16:49Z

btw unittests are failing because it can't find the nc_open_mem method, not sure if this should be dynamically looked up or bump up the minimum netCDF version required

jswhit · 2017-04-28T19:50:15Z

This is great, thanks!

To fix the issue with the tests, take a look at the check_api function in setup.py. You can add a check there to see if the include file has nc_open_mem. setup.py should then modify include/constants.pyx to add DEF HAS_NC_OPEN_MEM = 1. Thenc_open_mem calls in _netCDF4.pyx can then be wrapped with IF HAS_NC_OPEN_MEM:.

thehesiod · 2017-04-28T19:51:14Z

btw, why is netCDF4/_netCDF4.c checked in? it's generated when setup.py is run.

jswhit · 2017-04-28T19:52:05Z

We'll also need a test.

_netCD4.c has to be manually generated. It should be created with all the values in include/constants.pyx set to 0. It's only used if cython is not installed.

jswhit · 2017-04-28T19:53:13Z

Probably we should just require cython and get rid of _netCDF.c in the repo.

matthew-brett · 2017-04-28T19:57:56Z

Cython is super-easy to install these days - there are wheels for many versions for OSX, Windows, Linux.

thehesiod · 2017-04-28T19:58:45Z

working on updates, btw mind if I PEP'ify some of the files? get a lot of warnings in PyCharm

thehesiod · 2017-04-28T22:49:16Z

ok, made nc_open_mem lookup dynamic and added a test.

thehesiod · 2017-04-28T22:51:12Z

btw, I'm not even sure how the .c file here helps because isn't it specific to a python version / platform / cython version? Not sure how that ever worked correctly. The one in this PR is definitely wrong so need to figure out how to create a "correct" one.

thehesiod · 2017-04-28T23:07:29Z

btw let me know how I should fix this new unittest error. I'm guessing it's compiling with all the settings off. Further it won't work until there's a version of netcdf-c with the fix

shoyer · 2017-04-28T23:10:17Z

Probably we should just require cython and get rid of _netCDF.c in the repo.

The standard way to do this is to generate C files for upload to pypi, but not include them in the source.
cytoolz has a relatively sane version of this setup: https://github.com/pytoolz/cytoolz/blob/master/setup.py

shoyer · 2017-04-28T23:12:04Z

netCDF4/_netCDF4.pyx

@@ -1712,7 +1721,7 @@ references to the parent Dataset or Group.
    the parent Dataset or Group.""" 

    def __init__(self, filename, mode='r', clobber=True, format='NETCDF4',


Can we make filename optional if memory is provided?

Even if netCDF-C needs a bogus filename argument, I imagine we could generate something at random.

It gets reflected in the Dataset.filepath attribute so I think we should keep it. My unittest actually validates this

OK, I guess so. It just seems pretty pointless to me :)

ya, weirdness :)

Some text should be added to the docstring noting that filename is only used to set the value of filepath when memory is specified.

jswhit · 2017-04-29T15:58:14Z

Looks like the _netCDF.c still uses nc_open_mem, so that will cause a failing test. To generate one that will work with the minimum version of the netcdf library, edit constants.pyx and set all the elements to zero, then cd _netCDF; cython -I../include _netCDF4.pyx, then commit the new version of _netCDF.c.

I'd like to remove _netCDF.c from the repo, but that should be a separate pull request.

Are the other failing tests due to the bug in Unidata/netcdf-c#394?

thehesiod · 2017-04-29T17:26:44Z

updated, so I think the tests are failing now because it's running the mem test with the feature disabled?

jswhit · 2017-04-29T21:46:38Z

The test fails on my machine even though I have 4.4.1 (with nc_open_mem). I guess this is because of Unidata/netcdf-c#394? If so, a solution would be to disable the test in run_all.py if __netcdf4libversion__ < 4.4.1.2 (assuming that the fix gets in the next release of netcdf-c).

thehesiod · 2017-04-30T07:18:11Z

btw, it seems like we should be able to use a memoryview insead reading http://docs.cython.org/en/latest/src/userguide/memoryviews.html?highlight=memory%20views.html. Let me know what you guys think

jswhit · 2017-04-30T13:24:58Z

netCDF4/_netCDF4.pyx

        """
        cdef int grpid, ierr, numgrps, numdims, numvars
        cdef char *path
        cdef char namstring[NC_MAX_NAME+1]
+
+        memset(&self._buffer, 0, sizeof(self._buffer))
+


Probably ought to add _buffer to _private_atts so it doesn't get mistaken for a netcdf attribute.

jswhit · 2017-04-30T13:26:06Z

Don't see any real benefit to using memoryviews, since we're not slicing or manipulating the buffer in any way.

jswhit · 2017-04-30T13:31:53Z

netCDF4/_netCDF4.pyx

 # so it will compile with versions <= 4.2.
 NC_DISKLESS = 0x0008
+NC_INMEMORY = 0x8000


Doesn't look like NC_INMEMORY is ever used

jswhit · 2017-04-30T13:34:46Z

test/tst_open_mem.py

+class TestOpenMem(unittest.TestCase):
+    def test_mem_open(self):
+        # Needs: https://github.com/Unidata/netcdf-c/pull/400
+        if netCDF4.__netcdf4libversion__ >= '4.4.1.2':


Why not just add this check in run_all.py so the test is skipped altogether?

actually updated the test to test different scenarios of this

jswhit · 2017-05-01T01:28:18Z

Merging now - thanks again for this contribution!

thehesiod · 2017-05-01T02:17:16Z

thank you for the quick responses! pleasure!

TWellman · 2017-07-11T17:36:40Z

Is there a working example/documentation based on this discussion for pulling a dataset via a url into memory using the newest netCDF4 version? I've read material on adding a memory=bytes construct in the init function to use the in-memory read capability from nc_open_mem(). However, my initial attempts have produced file location errors - "No such file or directory". I also ran a portion of the test script provided at https://github.com/Unidata/netcdf4-python/blob/master/test/tst_open_mem.py. An error was raised for netCDF4.netcdf4libversion = '4.4.1.1' (< 4.4.1.2). Do I need to pull from Unidata/netcdf-c#400 and integrate netcdf library changes directly into the package? Clarification is appreciated!

TWellman · 2017-07-11T17:41:43Z

I am testing on a Jupyter notebook using Python 3.5 and netCDF4 version 1.3.0, but could switch if needed.

jswhit · 2017-07-11T18:11:13Z

I think you need to link against the updated netcdf-c library. This one should work:

https://github.com/Unidata/netcdf-c/releases/tag/v4.5.0-rc1

dopplershift · 2017-07-11T19:36:04Z

Another option is to pass it a filename to an empty file, like from tempfile.NamedTemporaryFile

TWellman · 2017-07-11T22:21:17Z

Great, substituting a completely unrelated local netCDF file as the "filename" but also passing a memory file in byte format proved to be successful. The returned file dimensions were consistent with the memory file rather than the unrelated local netCDF file with different dimensions/variables. So that checks out.

I also attempted unsuccessfully to use the temp file option using FILE_NAME = tempfile.NamedTemporaryFile(suffix='.nc', delete=False).name, mainly because it is much cleaner in my view. However, using a temp file as prescribed returned an OSError: NetCDF: Unknown file format. Should I format the temp file in a specific way? Thank you.

thehesiod · 2017-07-11T22:22:33Z

I think "filename" just needs to be non-empty string

TWellman · 2017-07-11T22:28:34Z

If I just input a text string the code returns an OSError: No such file or directory, e.g. using nc_file = netCDF4.Dataset('temp_file', mode = 'r', memory=bytefile)

thehesiod · 2017-07-11T22:29:49Z

what version of netcdf-c ?

dopplershift · 2017-07-11T22:32:26Z

@TWellman I needed it to be a netCDF file (apparently--I guess I forgot about it). This is what I've done for now until netCDF 4.5 is released:

    with NamedTemporaryFile(suffix='.nc') as temp:
        # Create a temporary netCDF file to work around bug in netCDF C 4.4.1.1
        # We shouldn't actually need any file on disk.
        nc_temp = Dataset(temp.name, 'w').close()
        nc =  Dataset(temp.name, memory=mem_buffer)

TWellman · 2017-07-11T22:32:30Z

I should probably upgrade as suggested earlier. I am using netCDF4.netcdf4libversion = '4.4.1.1', netCDF4 version 1.3.0.

TWellman · 2017-07-11T22:34:25Z

Great, thank you!

add support for opening file from memory

3911a68

thehesiod mentioned this pull request Apr 28, 2017

Reading Dataset from memory #406

Closed

changes based on review

ec36dfa

shoyer reviewed Apr 28, 2017

View reviewed changes

build w/ everything disabled

7af0a28

thehesiod added 6 commits April 29, 2017 18:43

only run test on versions which have fix for nc_open_mem

0a32144

update ver

ab20386

returns void

ec5374f

correctly initialize the Py_Buffer

52c4088

revert wrong behavior

8b7bdbb

create common close method to avoid leaks

3553994

thehesiod mentioned this pull request Apr 30, 2017

Add per Dataset encoding support #655

Closed

jswhit reviewed Apr 30, 2017

View reviewed changes

thehesiod added 3 commits April 30, 2017 09:55

updates based on review

edfbf75

update based on review

8d32185

regen .c file

f7e02e2

jswhit merged commit 37f582b into Unidata:master May 1, 2017

rabernat mentioned this pull request Jun 9, 2017

Support creating DataSet from streaming object pydata/xarray#1075

Closed

tovogt mentioned this pull request Jan 21, 2022

[FEATURE]: Support reading/writing NetCDF4 from/to buffer (and .nc.gz) pydata/xarray#6181

Open

zerothi mentioned this pull request Jan 25, 2023

Using sisl to parse aiida-siesta output zerothi/sisl#484

Closed

		@@ -1712,7 +1721,7 @@ references to the parent Dataset or Group.
		the parent Dataset or Group."""

		def __init__(self, filename, mode='r', clobber=True, format='NETCDF4',

add support for opening file from memory #652

add support for opening file from memory #652

Conversation

thehesiod commented Apr 28, 2017 • edited Loading

thehesiod commented Apr 28, 2017 • edited Loading

jswhit commented Apr 28, 2017 • edited Loading

thehesiod commented Apr 28, 2017

jswhit commented Apr 28, 2017

jswhit commented Apr 28, 2017

matthew-brett commented Apr 28, 2017

thehesiod commented Apr 28, 2017

thehesiod commented Apr 28, 2017

thehesiod commented Apr 28, 2017 • edited Loading

thehesiod commented Apr 28, 2017

shoyer commented Apr 28, 2017

Choose a reason for hiding this comment

thehesiod Apr 28, 2017 • edited Loading

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

jswhit commented Apr 29, 2017

thehesiod commented Apr 29, 2017 • edited Loading

jswhit commented Apr 29, 2017 • edited Loading

thehesiod commented Apr 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jswhit commented Apr 30, 2017

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

jswhit commented May 1, 2017

thehesiod commented May 1, 2017

TWellman commented Jul 11, 2017

TWellman commented Jul 11, 2017

jswhit commented Jul 11, 2017

dopplershift commented Jul 11, 2017

TWellman commented Jul 11, 2017

thehesiod commented Jul 11, 2017

TWellman commented Jul 11, 2017

thehesiod commented Jul 11, 2017

dopplershift commented Jul 11, 2017

TWellman commented Jul 11, 2017

TWellman commented Jul 11, 2017

thehesiod commented Apr 28, 2017 •

edited

Loading

thehesiod commented Apr 28, 2017 •

edited

Loading

jswhit commented Apr 28, 2017 •

edited

Loading

thehesiod commented Apr 28, 2017 •

edited

Loading

thehesiod Apr 28, 2017 •

edited

Loading

thehesiod commented Apr 29, 2017 •

edited

Loading

jswhit commented Apr 29, 2017 •

edited

Loading