Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing property raises exception in 1.5.0 #511

Closed
matham opened this issue Jun 30, 2021 · 5 comments · Fixed by #513
Closed

Changing property raises exception in 1.5.0 #511

matham opened this issue Jun 30, 2021 · 5 comments · Fixed by #513
Labels

Comments

@matham
Copy link
Member

matham commented Jun 30, 2021

I'm testing out 1.5.0 and I ran into the following issue:

I have code that looks like:

f = nix.File.open(filename, nix.FileMode.ReadWrite)

if 'app_logs' not in f.sections:
    f.create_section('app_logs', 'log')
    f.sections['app_logs']['notes'] = ''

if f.sections['app_logs']['notes'] and notes:
    f.sections['app_logs']['notes'] += '\n'
f.sections['app_logs']['notes'] += notes

This worked fine in 1.4.9. But with 1.5.0 I get the following error from my pytest tests:

>       merger.merge_data(filename, alignment, notes='app notes')

ceed\tests\test_app\test_experiment.py:346:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ceed\analysis\merge_data.py:1635: in merge_data
    f.sections['app_logs']['notes'] += notes
e:\python\python37\lib\site-packages\nixio\section.py:463: in __setitem__
    prop.values = data
e:\python\python37\lib\site-packages\nixio\property.py:276: in values
    self._h5dataset.shape = np.shape(vals)
e:\python\python37\lib\site-packages\nixio\hdf5\h5dataset.py:109: in shape
    self.dataset.resize(shape)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <HDF5 dataset "notes": shape (1,), type "|O">, size = (1,), axis = None

    def resize(self, size, axis=None):
        """ Resize the dataset, or the specified axis.

        The dataset must be stored in chunked format; it can be resized up to
        the "maximum shape" (keyword maxshape) specified at creation time.
        The rank of the dataset cannot be changed.

        "Size" should be a shape tuple, or if an axis is specified, an integer.

        BEWARE: This functions differently than the NumPy resize() method!
        The data is not "reshuffled" to fit in the new shape; each axis is
        grown or shrunk independently.  The coordinates of existing data are
        fixed.
        """
        with phil:
            if self.chunks is None:
>               raise TypeError("Only chunked datasets can be resized")
E               TypeError: Only chunked datasets can be resized

e:\python\python37\lib\site-packages\h5py\_hl\dataset.py:425: TypeError

I imagine this has something to do with #432, but I don't fully understand how/if that disables chunking. Is there perhaps an alternate way to append to a property?

@jgrewe
Copy link
Member

jgrewe commented Jun 30, 2021

I am not sure, I can't reproduce the error. Do you intend to append a new value to the notes Property or do you want to extend the string stored in the Property?
In my example I do not get an error and running the code multiple times does as expected and appends the same "notes" to the existing string.

filename = "test.nix"
notes = "some_notes"
f = nixio.File.open(filename, nixio.FileMode.ReadWrite)

if 'app_logs' not in f.sections:
    f.create_section('app_logs', 'log')
    f.sections['app_logs']['notes'] = ''

if f.sections['app_logs']['notes'] and notes:
    f.sections['app_logs']['notes'] += '\n'
f.sections['app_logs']['notes'] += notes

print(f.sections['app_logs'].props['notes'], len(f.sections['app_logs'].props['notes'].values))
print(f.sections['app_logs']['notes'])

f.close()

@matham
Copy link
Member Author

matham commented Jun 30, 2021

Aha! This only happens if the original is created in 1.4.9 and upgraded to 1.5.0 and then the text is appended. E.g.:

filename = 'test.h5'


def run_in_1_4_9():
    f = nix.File.open(filename, nix.FileMode.Overwrite)
    f.create_section('app_logs', 'log')
    f.sections['app_logs']['notes'] = ''
    f.close()


def run_in_1_5_0():
    f = nix.File.open(filename, nix.FileMode.ReadWrite)
    f.sections['app_logs']['notes'] += 'app notes'
    f.close()


if __name__ == '__main__':
    run_in_1_4_9()
    # run_in_1_5_0()

Then, in 1.4.9 run run_in_1_4_9(). Then upgrade nixio to 1.5.0, run nixio upgrade test.h5 -f in the terminal and then run run_in_1_5_0() and the error will appear.

My goal is to extend the string to make the string larger.

@matham
Copy link
Member Author

matham commented Jun 30, 2021

If I change in the upgrade.py file prop = hfile.create_dataset(name, dtype=dtype, data=data, chunks=True) by adding chunks=True, it seems to work. Not sure why this is required as new properties don't seem to have that keyword.

@jgrewe jgrewe added the bug label Jun 30, 2021
@jgrewe
Copy link
Member

jgrewe commented Jun 30, 2021

ah, this is a good catch, thank you! Maybe @achilleas-k has some insights into this.

@achilleas-k
Copy link
Member

achilleas-k commented Jul 1, 2021

If I change in the upgrade.py file prop = hfile.create_dataset(name, dtype=dtype, data=data, chunks=True) by adding chunks=True, it seems to work. Not sure why this is required as new properties don't seem to have that keyword.

You're right. Datasets should be created with chunked storage:

self.dataset = self._parent.require_dataset(
name, shape=shape, dtype=dtype, chunks=True, maxshape=maxshape,
**comprargs
)

This was also true in v1.4.9. The converter/upgrade script should do the same.

EDIT: If the storage isn't chunked and the size of a dataset isn't specified as dynamic, then datasets have fixed size by default, which is why you can't append to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants