-
Notifications
You must be signed in to change notification settings - Fork 2
fix #85 : display dtype (in title) #86
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
fix #85 : display dtype (in title) #86
Conversation
* display shape + dtype of Numpy ndarray in title
larray_editor/editor.py
Outdated
shape = ' x '.join('({})'.format(length) for length in array.shape) | ||
dtype = ' [{}]'.format(array.dtype.name) | ||
title += [(name + ': ' + shape + dtype) if name else shape + dtype] | ||
# name of non-LArray/Numpy displayed item (if not None) | ||
elif name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this case is even possible now that you explicitly handle np.ndarray.
larray_editor/editor.py
Outdated
shape = ' x '.join('({})'.format(length) for length in array.shape) | ||
dtype = ' [{}]'.format(array.dtype.name) | ||
title += [(name + ': ' + shape + dtype) if name else shape + dtype] | ||
# name of non-LArray/Numpy displayed item (if not None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not move the dtype thing outside this if statement:
if isinstance(array, LArray):
...
elif isinstance(array, np.ndarray):
...
else:
...
if isinstance(array, (LArray, np.ndarray)):
title[-1] += ' [{}]'.format(array.dtype.name)
also anything we display in the viewer should have a shape (even if no .shape attribute), so you could refactor this into something like:
if isinstance(array, LArray):
shape_parts = ["%s (%d)" % (display_name, len(axis)) for display_name, axis in zip(axes.display_names, axes)]
elif isinstance(array, np.ndarray):
shape_parts = [str(l) for l in array.shape]
else:
# unsure this is necessary, but we could add this case
shape_parts = [str(len(array))]
array_info = ' x '.join(shape_parts)
if name:
array_info = name + ': ' array_info
if isinstance(...):
array_info += ' [{}]'.format(array.dtype.name)
full_title = ' - '.join([title_part for title_part in [fname, array_info, title] if title_part != ''])
BTW: I think that the unsaved_modification indicator would look better/be more standard if it was next to (just before) the filename, instead of at the end.
0c66433
to
d7f5b43
Compare
larray_editor/editor.py
Outdated
title = [name] | ||
shape = ['{} ({})'.format(display_name, len(axis)) | ||
for display_name, axis in zip(array.axes.display_names, array.axes)] | ||
# if it's not an LArray, it must be a Numpy ndarray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding an assert checking that assumption would probably be worth it in the long run, when we start displaying more stuff
fix larray-project#85 : display dtype in title
No need to change Tooltip associated with listwidget. It uses
array.info
which will be changed in larray project (see PR larray-project/larray#459).