Fix shape and length reporting of gridded interfaces for Python 3.6 on Win 64

[ceball]

Part of #2895 (this PR just covers grid-based interfaces).

I used the below test files to check these changes. After switching to pyctdev and getting windows tests, these tests could be included in hv's test suite (but presumably re-written into class-based unit tests, which would reduce some boilerplate overlap). (Note for the future: the tests are intentionally set up to work on a machine with less than 4GB ram, for travis/appveyor.)

test_grid.py:

import numpy as np, holoviews as hv
shape = (100, 1+np.iinfo(np.int32).max//100)
img = hv.Image( (range(shape[1]), range(shape[0]), np.empty(shape,dtype=np.uint8)), datatype=['grid'])
img = img.clone(datatype=['grid'])

print(shape, img.shape, type(img.shape[0]))
s = shape[0]*shape[1]

grid_shape = img.interface.shape(img)
# fails because Grid.shape() is wrong
assert grid_shape[0] == s, "%s != %s"%(grid_shape[0],s)

test_image.py:

import numpy as np, holoviews as hv
shape = (100, 1+np.iinfo(np.int32).max//100)
img = hv.Image(np.ndarray(shape,dtype=np.uint8))


print(shape, img.data.shape, type(img.data.shape[0]), img.shape, type(img.shape[0]))
s = shape[0]*shape[1]

length = img.interface.length(img)
# fails because Image.length() is wrong
assert length==s, "%s != %s"%(length,s)
assert img.shape[0]==s, "%s != %s"%(img.shape[0],s)

test_iris.py:

import numpy as np, holoviews as hv
shape = (100, 1+np.iinfo(np.int32).max//100)
img = hv.Image( (range(shape[1]), range(shape[0]), np.empty(shape,dtype=np.uint8)), datatype=['grid'])
img = img.clone(datatype=['cube'])

print(shape, img.data.shape, type(img.data.shape[0]), img.shape, type(img.shape[0]))
s = shape[0]*shape[1]

# fails because CubeInterface.length() is wrong
assert img.shape[0]==s, "%s != %s"%(img.shape[0],s)

test_xarray.py:

import numpy as np, holoviews as hv
shape = (100, 1+np.iinfo(np.int32).max//100)
img = hv.Image( (range(shape[1]), range(shape[0]), np.empty(shape,dtype=np.uint8)), datatype=['grid'])
img = img.clone(datatype=['xarray'])

print(shape, img.shape, type(img.shape[0]))
s = shape[0]*shape[1]

# fails because XArrayInterface.shape() is wrong
assert img.shape[0]==s, "%s != %s"%(img.shape[0],s)
# fails because XArrayInterface.length() is wrong
assert img.interface.length(img)==img.shape[0] # TODO verify that definition of length is right

[philippjfr]

Prefer to leave a space between arguments. Same applies below.

[ceball]

Sorry, will fix those.

hv has flake8 whitespace checking turned off on travis. I also couldn't see any instructions for contributors about whitespace (whether it's important, and if so, what the rules are). And when I skimmed the hv code, I saw inconsistent whitespace in general, including spacing between arguments. So I think that makes it a bit tricky for contributors to get whitespace right, unless I've failed to see something?

If you can point me to the rules hv's using, I'll make a PR in docs and/or testing, plus clean up the existing whitespace.

[philippjfr]

We don't have consistent rules (although I think we really should), but that's a whole argument to have. That said I'm very strongly in favor of separating arguments with whitespace and would be in favor of enabling this particular rule as a flake8 check immediately.

[philippjfr]

Looks good to me apart from a minor code formatting issue.

[ceball]

Are the test failures my fault? I'll investigate if so (I couldn't run the test suite locally on the machine I was using for this PR).

[philippjfr]

The errors are pretty bizarre, it looks like the Dimension label parameters are set to None now when they should just mirror the name by default. Can't see what that has to do with the PR though. Is the PR otherwise up-to-date with master?

[ceball]

Is the PR otherwise up-to-date with master?

Probably not, as I did most of this last week. I'll investigate!

Also so far I only ran my own tests on these changes, not the existing HV tests (and I only ran on windows).

[philippjfr]

Don't think anything would have changed since last week. Really not sure what could have changed though.

[jlstevens]

Seeing label set to None is worrying indeed. How can that suddenly break?

[ceball]

I had a horrible thought on my way home...the errors somehow look like they'd be something more to do with recent param changes (rather than anything on this PR). However, I've just checked, and those changes to param were released before the end of June. There must have been successful tests of hv since then, right...?

[ceball]

Hmm, my understanding had always been that holoviews is using param master on travis, but it's not. holoviews on travis gets its dependencies from either conda defaults, conda conda-forge, conda bokeh, or easy_install (via python setup.py develop) - varies by dependency. param's coming from conda defaults. And looks like param 1.7 made it to defaults 7 days ago. There hasn't been a successful travis build of holoviews since then, as far as I can tell.

[ceball]

I've just pushed a commit to test that theory (I'm away for a while - will come back later).

[philippjfr]

param's coming from conda defaults. And looks like param 1.7 made it to defaults 7 days ago. There hasn't been a successful travis build of holoviews since then, as far as I can tell.

That's pretty worrying, any idea what might have changed in 1.7?

[ceball]

I was imagining something to do with the namespace stuff. I should be able to look later (I'm away now).

…

On Tue, 31 Jul 2018, 18:20 Philipp Rudiger, ***@***.***> wrote: param's coming from conda defaults. And looks like param 1.7 made it to defaults 7 days ago. There hasn't been a successful travis build of holoviews since then, as far as I can tell. That's pretty worrying, any idea what might have changed in 1.7? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2903 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAHiUJt_VebQ2SmQ5fX4dfXT_-BTsW7ks5uMJHLgaJpZM4VoAK2> .

[jlstevens]

That is worrying indeed. That said, wouldn't param 1.7 have been pulled in for the scipy pyviz tutorial? It seems to have worked fine then...

[ceball]

Preliminary thoughts...

The problem appears to be with the unpickled reference data, i.e. with backwards compatibility of pickles, rather than a problem with "current" hv or param.

On unpickling, the recently added param namespace object is not being set up correctly from the state in the pickle (of the old param). Specifically, the param object's self isn't set (so is None). That of course leads to various problems (e.g. get_param_values(), used in hv's comparisons, reports the class's values rather than the instance's values).

To test that idea out, I thought it'd be a one line fix in Parameterized.setstate to set self on the param object, but one or more hv things (e.g. LabelledData) appear to have their own independent state handling for pickle, and don't use Parameterized's, so I need to discuss if that's deliberate or an oversight. (Which I can do once I verify this really is the problem by creating a test case.)

I also need to see what param's current policy is regarding backwards compatibility of pickles (and consider if that's how it should be). I used to make pickle loading backwards compatible with old pickles when param was part of topographica, but not sure if I ever moved any of that stuff to param itself, or what's happened since then...

[ceball]

About the tests failing: moved to #2914

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.