Extend __str__ and __repr__ methods of BaseTableCoordinate, ExtraCoords, GlobalCoords

[dhomeier]

Description

This is a WIP to improve and expand the string representations to begin addressing #413.
For a start adding physical_types information to ExtraCoords and get line breaks better in line with numpy standards for long arrays.

>>> cube.extra_coords
<ndcube.extra_coords.extra_coords.ExtraCoords object at 0x121c60b80>
ExtraCoords(time (0) ['custom:time:creation']: ['2310-03-22T12:30:30.990' '2416-03-22T12:30:32.000'
 '2416-03-23T12:30:33.000'],
            exposure time (0) ['custom:time:duration']: (<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] s>,))
>>> np.set_printoptions(linewidth=160)
>>> ndc.extra_coords
<ndcube.extra_coords.extra_coords.ExtraCoords object at 0x121c60b80>
ExtraCoords(time (0) ['custom:time:creation']: ['2310-03-22T12:30:30.990' '2416-03-22T12:30:32.000' '2416-03-23T12:30:33.000'],
            exposure time (0) ['custom:time:duration']: (<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] s>,))
>>> np.set_printoptions(linewidth=300)
>>> ndc.extra_coords
<ndcube.extra_coords.extra_coords.ExtraCoords object at 0x121c60b80>
ExtraCoords(time (0) ['custom:time:creation']: ['2310-03-22T12:30:30.990' '2416-03-22T12:30:32.000' '2416-03-23T12:30:33.000'], exposure time (0) ['custom:time:duration']: (<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] s>,))

The output could still be slightly prettified for empty physical_types, but having None printed might be useful, too.

Some information on NDCube.data should be added next; do we want to keep this at metadata or also print the content in a similar manner?

[DanRyanIrish]

Hi @dhomeier. Thanks so much for tackling this issue. Any more progress recently?

[pep8speaks]

Hello @dhomeier! Thanks for updating this PR.

• In the file ndcube/extra_coords/table_coord.py:

Line 477:56: W504 line break after binary operator
Line 242:48: W504 line break after binary operator

• In the file ndcube/global_coords.py:

Line 70:70: W504 line break after binary operator

• In the file ndcube/ndcube.py:

Line 661:38: E741 ambiguous variable name 'l'

Comment last updated at 2021-08-13 13:46:03 UTC

[DanRyanIrish]

Suggested change

	Data: Array{self.data.shape} [{self.unit}] '{self.data.dtype}'""")
	Unit: {self.unit}
	Data Type: {self.data.dtype}""")

I suggest his change because the data shape is already captured by the dimensions line, above.

[DanRyanIrish]

I like what you've done wit ExtraCoords and GlobalCoords @dhomeier . My one question is what happens if an coord is very long? Is the __str__ still readable?

[DanRyanIrish]

This should have a changelog file named 453.doc.rst in the changelog directory.

[DanRyanIrish]

See what my comment today on #413 for a roadmap for this PR.

[dhomeier]

I like what you've done wit ExtraCoords and GlobalCoords @dhomeier . My one question is what happens if an coord is very long? Is the __str__ still readable?

That would be handled by the Quantity / numpy print settings, e.g. with np.set_printoptions(threshold=300, linewidth=100)

GlobalCoords(length ['custom:physical_type1']:
<Quantity [400., 401., 402., ..., 897., 898., 899.] nm>,
             time ['custom:physical_type2']:
<Quantity [ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9., 10., 11., 12., 13., 14., 15., 16., 17.,
           18., 19., 20., 21., 22., 23., 24., 25., 26., 27., 28., 29., 30., 31., 32., 33., 34., 35.,
           36., 37., 38., 39.] s>)

I have not tried to match the indentation level for the values part but rather left that to Quantity; indenting additionally to the GlobalCoords position would also consume too much space imo.

Or did you mean very long (multiple?) physical_type strings?

[DanRyanIrish]

No, that's exactly what I meant. I think your decision makes sense.

[dhomeier]

I followed the same for the *TableCoordinate representations, slightly inconsistent indenting but hopefully readable.
I also just stripped the tuple representation of self.table as it introduced further offsets; for a BaseTableCoordinate with multiple quantities it's still not wrapping well; might improve this further with manually putting the elements together as in ExtraCoords, but I think NDCubeSequence should have higher priority now.

<ndcube.extra_coords.table_coord.QuantityTableCoordinate object at 0x107a2c670>
QuantityTableCoordinate ['length', 'width'] ['custom:distance1', 'custom:distance2']:
<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] km>, <Quantity [ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9., 10., 11., 12., 13.,
           14., 15., 16., 17., 18., 19., 20., 21., 22., 23., 24., 25., 26., 27.,
           28., 29., 30., 31., 32., 33., 34., 35., 36., 37., 38., 39., 40., 41.,
           42., 43., 44., 45., 46., 47., 48., 49.] m>

<ndcube.extra_coords.table_coord.MultipleTableCoordinate object at 0x107b61d00>
MultipleTableCoordinate(tables=[QuantityTableCoordinate [None]:
<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] km>, <Quantity [ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9., 10., 11., 12.,
           13., 14., 15., 16., 17., 18., 19., 20., 21., 22., 23., 24., 25.,
           26., 27., 28., 29., 30., 31., 32., 33., 34., 35., 36., 37., 38.,
           39., 40., 41., 42., 43., 44., 45., 46., 47., 48., 49.] m>,
                                QuantityTableCoordinate [None]:
<Quantity [ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9., 10., 11., 12.,
           13., 14., 15., 16., 17., 18., 19.] s>])

[DanRyanIrish]

Keep up the good work!

[Cadair]

Some information on NDCube.data should be added next; do we want to keep this at metadata or also print the content in a similar manner?

I don't think printing the content, or in fact doing anything which makes it assume it's a numpy array is a good idea. Printing the actual type of the array i.e. dask vs numpy might be useful though?

[wtbarnes]

I've been meaning to comment on this to say this would be a good opportunity to have support for the dask html repr in cases where the underlying array is a dask array. I'm not sure you would even need to special case for dask. You could presumably just pass through the html repr for the underlying array if it exists.

[Cadair]

Yeah that was one of the things I was hoping to achieve as well.

[dhomeier]

Thanks for the comments on Dask arrays; I did have the pointer to its _repr_html but wasn't sure if we wanted also a method for that or just equivalent information through the __str__ and __repr__ methods. Adding a thin wrapper should in fact be quite easy; constructing something similar for ndarray probably feasible. Though looking at the examples in dask/dask#7763 maybe the question is how much there is to display for plain ndarray data.

But in fact I had not realised that NDCube.data would take Dask arrays as input objects; the general docs imply that it might accept anything (sufficiently) "ndarray-like", but the API reference states numpy.ndarray. nddata also does not advertise this very clearly, though it looks again like anything indexable with shape and __array__ attributes might get in untouched.
Do you have any further pointers where or how such usage is implemented?

[Cadair]

Do you have any further pointers where or how such usage is implemented?

We pretty much don't do anything with the array at all in most places, so any "array-like" should work. The only real exceptions to this are reproject and the upcoming superpixel methods where we are actually modifying the data. If there are obvious cases in the docs where we are saying numpy array we should change that, and probably make more of a point of selling (and maintaining) good dask support.

[DanRyanIrish]

@dhomeier, FYI I posted a comment relevant to this PR on the associated issue, #413

[dhomeier]

We pretty much don't do anything with the array at all in most places, so any "array-like" should work. The only real exceptions

Sounds reasonable. I certainly don't think the representations would need anything beyond that. I am a bit less sure what other (inherited) NDData methods might use additional ndarray attributes.

[Cadair]

Yeah it's decidedly possible that subclasses will assume ndarray, but that's ok :D

[dhomeier]

I was thinking more about methods people might use that NDCube has inherited, and not overwritten, from NDData potentially using other ndarray attributes. But the only instance springing to my eye was in fact __repr__ using np.array2string in the latter... ;-)

[Cadair]

Yeah, the __repr__ one is something I meant to patch in astropy a long time ago, and was the original motivation for overriding it in NDCube!

I don't think there are any other examples, but if there are we should fix them.

[dhomeier]

Are there any tests using dask arrays as data?

[Cadair]

I am not sure, I don't think they are that comprehensive if they exist.

[dhomeier]

Marking this as ready now, since any real overhaul of NDCube depends on #474, so this should work as is for the classes covered here.

[Cadair]

🤔 Do we really have no examples of the __repr__ or __str__ methods in the docs at all? This surprises me. (Also means I can't see any examples of the output 🤣 )

[dhomeier]

Quite possible; not sure what documentation is needed on how to use it (it's what pops up when you just type cube.extra_coords anyway). 😜
What was not clear to me before I started this is the difference between __str__ and __repr__, but that is mostly generic Python knowledge.