Shapes in object representations are printed as lists instead of tuples

[colleenjg]

When printing different torch_brain objects, the shape of different attributes are shown, e.g.

behavior

prints:

RegularTimeSeries(
  hand_vel=[1000, 2],
  eye_pos=[1000, 2],
  pupil_size=[1000]
)

In a few cases (especially when printing an interval), I found this confusing. It looked to me like I was seeing the content of the attribute, instead of its shape. Since array shapes are typically printed as tuples in python, might we consider changing this to:

RegularTimeSeries(
  hand_vel=(1000, 2),
  eye_pos=(1000, 2),
  pupil_size=(1000, )
)

[vinamarora8]

Good point! I guess we should change it both in the code (the repr methods) and in the docs.

[vinamarora8]

Maybe having the = adds to the confusion? Maybe we could represent the whole object like this?

RegularTimeSeries(
  hand_vel: array, shape (1000, 2),
  eye_pos: array, shape (1000, 2),
  pupil_size: array, shape (1000, )
)

[colleenjg]

I think that's a good idea! Shall I make a PR?

[vinamarora8]

Yes @colleenjg feel free to!

[colleenjg]

Once I make the changes, there will be quite a lot of documentation to update. So first, I want to get a consensus on the changes to make. Let me know what you think!

Proposed changes:

• add data type and clearly indicate shape (shown as tuple) => from some testing, I found shape=(3, 48) was more easily interpreted for a complex Data object with lots of components than shape (3, 48). Let me know what you prefer.
• use ":" when the value of a key is described (e.g., timestamps: array, shape=(3,)), and "=" when the it is printed (e.g., _absolute_start=1.0
• add an indentation to the first level of the Data object repr
• add a trailing , to the last component of each ArrayDict object (the Data object had it)

Before:

>>> data.slice(1, 3)

Data(
session_id='session_0',
spikes=IrregularTimeSeries(
  timestamps=[3],
  unit_index=[3],
  waveforms=[3, 48]
),
lfp=RegularTimeSeries(
  raw=[500, 3]
),
units=ArrayDict(
  id=[3],
  brain_region=[3]
),
trials=Interval(
  start=[2],
  end=[2],
  go_cue_time=[2],
  drifting_gratings_dir=[2]
),
drifting_gratings_imgs=[8, 3, 32, 32],
_absolute_start=1.0,
)

After:

>>> data.slice(1, 3)

Data(
  session_id='session_0',
  spikes=IrregularTimeSeries(
    timestamps: array, shape=(3,),
    unit_index: array, shape=(3,),
    waveforms: array, shape=(3, 48),
  ),
  lfp=RegularTimeSeries(
    raw: array, shape=(500, 3),
  ),
  units=ArrayDict(
    id: array, shape=(3,),
    brain_region: array, shape=(3,),
  ),
  trials=Interval(
    start: array, shape=(2,),
    end: array, shape=(2,),
    go_cue_time: array, shape=(2,),
    drifting_gratings_dir: array, shape=(2,),
  ),
  drifting_gratings_imgs: array, shape=(8, 3, 32, 32),
  _absolute_start=1.0,
)

[colleenjg]

Though, I think for lazy loaded objects, the arraydicts print as:

LazyRegularTimeSeries(
  pos=<HDF5 dataset "pos": shape (190000,), type "<f8">,
  speed=<HDF5 dataset "speed": shape (190000,), type "<f8">,
  vel=<HDF5 dataset "vel": shape (190000,), type "<f8">
) 

with no = after shape. So it might be worth just matching that and removing the = above for the shapes.

[milosobral]

I think shape (3, ) is better than shape=(3,), especially if it matches lazy loaded objects.

[vinamarora8]

I found shape=(3, 48) was more easily interpreted for a complex Data object with lots of components than shape (3, 48)

I agree w this.

We could change how the lazy objects are repr'd too. Like pos: lazy array, shape=(x, y, z)?

[colleenjg]

Here is an updated version where I show what happens for a data array loaded from an h5 file.

Data(
  drifting_gratings_imgs: array, shape=(8, 3, 32, 32), dtype=float64,
  lfp=LazyRegularTimeSeries(
    raw: lazy array, shape=(1000, 3), dtype=float64,
  ),
  spikes=LazyIrregularTimeSeries(
    timestamps: lazy array, shape=(6,), dtype=float64,
    unit_index: lazy array, shape=(6,), dtype=int64,
    waveforms: lazy array, shape=(6, 48), dtype=float64,
  ),
  trials=LazyInterval(
    drifting_gratings_dir: lazy array, shape=(3,), dtype=int64,
    end: lazy array, shape=(3,), dtype=int64,
    go_cue_time: lazy array, shape=(3,), dtype=float64,
    start: lazy array, shape=(3,), dtype=int64,
  ),
  units=LazyArrayDict(
    brain_region: lazy array, shape=(3,), dtype=|S3,
    id: lazy array, shape=(3,), dtype=|S6,
  ),
  session_id='session_0',
  _absolute_start=0.0,
)

You'll see that I've added the datatype, but only for arrays (array or lazy array). I don't recall from our discussion whether we decided to include or omit datatype.

Here is without datatype, for visual comparison:

Data(
  drifting_gratings_imgs: array, shape=(8, 3, 32, 32),
  lfp=LazyRegularTimeSeries(
    raw: lazy array, shape=(1000, 3),
  ),
  spikes=LazyIrregularTimeSeries(
    timestamps: lazy array, shape=(6,),
    unit_index: lazy array, shape=(6,),
    waveforms: lazy array, shape=(6, 48),
  ),
  trials=LazyInterval(
    drifting_gratings_dir: lazy array, shape=(3,),
    end: lazy array, shape=(3,),
    go_cue_time: lazy array, shape=(3,),
    start: lazy array, shape=(3,),
  ),
  units=LazyArrayDict(
    brain_region: lazy array, shape=(3,),
    id: lazy array, shape=(3,),
  ),
  session_id='session_0',
  _absolute_start=0.0,
)

Note: Having the dtype shown does expose an issue in this example (which I point to in #347), which is that the trials interval was specified using a list of ints for start and end. The data object creation throws a warning, and then looking at the repr would expose where the issue is - for what it's worth.

[AlexandreAndr]

Hi! I like a lot the updated version and I would vote to have the dtype (it can be a useful information).
Curious about these types |S6 and |S3, are they strings from h5? Should we keep them or should we match to something else?

[vinamarora8]

The updated version looks good to me too! I think we decided to include the dtype here.

[colleenjg]

Hi! I like a lot the updated version and I would vote to have the dtype (it can be a useful information). Curious about these types |S6 and |S3, are they strings from h5? Should we keep them or should we match to something else?

Hmm. That's a good catch. So the fully loaded version will print as:

  units=ArrayDict(
    id: array, shape=(3,), dtype=<U6,
    brain_region: array, shape=(3,), dtype=<U3,

and the lazy loaded version printed as:

  units=LazyArrayDict(
    brain_region: lazy array, shape=(3,), dtype=|S3,
    id: lazy array, shape=(3,), dtype=|S6,

Converting the h5 dtype to np.dtype doesn't change it. These string array are saved as byte strings, but converted from and to Unicode strings when saved and loaded, respectively. I think in most cases, the conversion will be simply -> |S -> <U, but it's conceivable that a different conversion could occur in some cases (if some strings contain non ASCII characters). And because the conversion is based on the longest string to encode, we can't infer it from loading just one item in the array.

I think unfortunately, the safest and most accurate thing to do is to report the current dtype of the data, e.g. |S3 when it is not loaded, and <U3 when it is loaded.