Reduce numpy array traceback print by JoOkuma · Pull Request #2910 · napari/napari

JoOkuma · 2021-06-23T22:02:51Z

Description

This PR changes the print behavior for NumPy arrays during traceback within napari.

Traceback before

flows_orig = array([[[0, 0, 0],
        [0, 0, 0],
        [0, 0, 0],
        ...,
        [0, 0, 0],
        [0, 0, 0],
        [0, 0, 0]],

       [[0, 0, 0],
        [0, 0, 0],
        [0, 0, 0],
        ...,
        [0, 0, 0],
        [0, 0, 0],
        [0, 0, 0]],

       [[0, 0, 0],
        [0, 0, 0],
        [0, 0, 0],
        ...,
        [0, 0, 0],
        [0, 0, 0],
        [0, 0, 0]],

       ...,

       [[0, 0, 0],
        [0, 0, 0],
        [0, 0, 0],
        ...,
        [0, 0, 0],
        [0, 0, 0],
        [0, 0, 0]],

       [[0, 0, 0],
        [0, 0, 0],
        [0, 0, 0],
        ...,
        [0, 0, 0],
        [0, 0, 0],
        [0, 0, 0]],

       [[0, 0, 0],
        [0, 0, 0],
        [0, 0, 0],
        ...,
        [0, 0, 0],
        [0, 0, 0],
        [0, 0, 0]]], dtype=uint8)

Traceback after

flows_orig = <<class 'numpy.ndarray'> (75, 658, 3) uint8>

Type of change

New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

References

The chosen string formatting imitates the zarr string representation.
The numpy function used is documented here. Unfortunately, it does not provide a context manager.
An alternative would be set_printoptions but the formatting changes are more limited.

How has this been tested?

all tests pass with my change on my machine

Final checklist:

My PR is the minimum possible work for the desired functionality
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
If I included new strings, I have used trans. to make them localizable.
For more information see our translations guide.

Note

I left some checkboxes unchecked because I'm not sure if they are relevant.
Additional suggestions are welcome.

tlambert03

works for me, thanks!

One possible elaboration would be to accept a parameter to format_exc_info that controls this behavior... then

add a checkbox to the QtPluginErrReport that lets the user control this.
pass the value of the checkbox to format_exceptions here
pass that value through to format_exc_info here

tlambert03 · 2021-06-23T22:13:28Z

napari/utils/_tracebacks.py

            info: ExcInfo, as_html: bool, color='Neutral'
        ) -> str:
+            # avoids printing the array data
+            np.set_string_function(


two thoughts:
a) is there a way to get the current string_function? (so that we can restore it in case a user has already set it?)

b) we probably want to be extra careful not to set this on someone and have it be permanent... so probably best to put everything between this line and the set_string_function(None) line in a try/finally clause to make sure that we set it back to whatever it was before

I could not find a way to get it back, I think it is not exposed to the NumPy Python API :(

yeah, bummer... some discussion here: numpy/numpy#11266

codecov · 2021-06-23T22:18:04Z

Codecov Report

Merging #2910 (a7a73f3) into master (ef717a3) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2910      +/-   ##
==========================================
- Coverage   82.97%   82.96%   -0.01%     
==========================================
  Files         500      500              
  Lines       41998    42020      +22     
==========================================
+ Hits        34846    34863      +17     
- Misses       7152     7157       +5

Impacted Files	Coverage Δ
napari/utils/_tracebacks.py	`89.65% <100.00%> (+0.90%)`	⬆️
napari/_qt/qt_event_loop.py	`60.76% <0.00%> (-2.14%)`	⬇️
napari/utils/action_manager.py	`89.24% <0.00%> (ø)`
napari/utils/_tests/test_translations.py	`97.02% <0.00%> (+0.07%)`	⬆️
napari/utils/translations.py	`92.39% <0.00%> (+0.13%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef717a3...a7a73f3. Read the comment docs.

JoOkuma · 2021-06-23T23:17:02Z

works for me, thanks!

One possible elaboration would be to accept a parameter to format_exc_info that controls this behavior... then

1. add a checkbox to the [`QtPluginErrReport`](https://github.com/napari/napari/blob/ef717a32056bff95772f60259f7e1eb74fdb2b76/napari/_qt/dialogs/qt_plugin_report.py#L23) that lets the user control this.

2. pass the value of the checkbox to `format_exceptions` [here](https://github.com/napari/napari/blob/master/napari/_qt/dialogs/qt_plugin_report.py#L162)

3. pass that value through to `format_exc_info` [here](https://github.com/napari/napari/blob/ef717a32056bff95772f60259f7e1eb74fdb2b76/napari/plugins/exceptions.py#L70)

Thanks for the suggestion. I will do that.

JoOkuma · 2021-06-24T00:22:23Z

Unfortunately, the state of the check box inside the QtPluginErrReport is not stored anywhere else so it does not affect the notifications and it resets every time we open it.
I'm not sure if it is a very helpful addition :(

tlambert03 · 2021-06-24T00:24:13Z

well, it could be added as a persistent setting... but it's fine if you want to leave that for (possibly) the future. if other @napari/core-devs are fine with this display mode (without having an option to change it) then it's fine with me

sofroniewn · 2021-06-24T05:24:43Z

Nice work! I havn't tried yet, but sounds great.

Unfortunately, the state of the check box inside the QtPluginErrReport is not stored anywhere else so it does not affect the notifications and it resets every time we open it.

well, it could be added as a persistent setting... but it's fine if you want to leave that for (possibly) the future. if other @napari/core-devs are fine with this display mode (without having an option to change it) then it's fine with me

Yeah, I might just drop the check box inside the QtPluginErrReport if not stored and just add the persistent setting. Shouldn't be too hard to add and might as well do now. @ppwadhwa can advise if you run into problems. I'd probably put in the Application panel

tlambert03 · 2021-06-24T10:03:13Z

It feels kinda weird to have to go into the preferences window to modify the appearance of a traceback in a different dialog don't you think? If it were in the plugin error dialog, one could check/uncheck it and immediately change the display (if they wanted to see the full array for some reason).

I'm not sure that every persistent setting necessarily needs to go into the main preferences window... and we could run into bloat like that

I think I'd probably even prefer the checkbox in the plugin error dialog without persistence over the standard preferences window with persistence... it's not a big deal to have to check it each time whenever you want to see the full array.

sofroniewn · 2021-06-24T17:47:50Z

It feels kinda weird to have to go into the preferences window to modify the appearance of a traceback in a different dialog don't you think?

Does this only effect tracebacks in the error dialog? Don't we also have a way to see tracebacks in the GUI in the bottom right and would this setting not effect them too?

I'm not sure that every persistent setting necessarily needs to go into the main preferences window... and we could run into bloat like that

I don't know, I think need to group and organize the settings to prevent bloat - so maybe this doesn't go in the main application setting, but some additional section, but I think i'd find it odd if i was trying to find a persistent setting and it wasn't in somewhere in main preferences window

I think I'd probably even prefer the checkbox in the plugin error dialog without persistence over the standard preferences window with persistence... it's not a big deal to have to check it each time whenever you want to see the full array.

Yeah that's fair.

My feelings about this are pretty weak right now, so @tlambert03 if you think this is good to go in now as is then I'm happy with it too. We can always revisit some of this stuff in the future. Let us know though if you want to see additional changes before merge, otherwise I'll put this in the awaiting merge section

tlambert03 · 2021-06-25T11:49:49Z

@JoOkuma

we chatted about this and are happy with the original functionality as you had it (i.e. no option to change the display, just always showing array shape and dtype). sorry for the runaround. am I correct in understanding that currently the checkbox effectively does nothing?
I'd say let's ignore persistence for the moment and do one of two things: either just remove the checkbox widget for now, or leave it in, but add a clicked callback that will re-render the traceback immediately (even though the check state will be lost when you close the window).

I'm fine with either of those options and leave it to you to decide. let us know when it's ready and we can merge. thanks again!

This reverts commit 0e7b6be.

JoOkuma · 2021-06-25T20:37:17Z

I reverted the changes so we don't have the checkbox anymore.
I think it would be most useful for runtime errors, but that wouldn't be the case.

I added some additional comments and extended the functionality to the traceback when ultratb is not available.

Thanks for the input.

tlambert03

perfect. thanks again!

# References and relevant issues * Closes #6752 * Reference https://numpy.org/devdocs/numpy_2_0_migration_guide.html#ruff-plugin * Previous related PR #2910 # Description This PR: 1. Adds a ruff linting rule to check compatibility with numpy v2.0, and 2. Updates existing napari code to ensure compatibility with numpy v2.0 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

changed numpy array traceback print

5151f75

github-actions bot assigned JoOkuma Jun 23, 2021

tlambert03 approved these changes Jun 23, 2021

View reviewed changes

tlambert03 reviewed Jun 23, 2021

View reviewed changes

added checkbox to reduced array formatting

0e7b6be

github-actions bot added the qt Relates to qt label Jun 24, 2021

sofroniewn added this to the 0.4.11 milestone Jun 24, 2021

JoOkuma added 2 commits June 25, 2021 13:28

Revert "added checkbox to reduced array formatting"

488787e

This reverts commit 0e7b6be.

reverting changes that added the checkbox

a7a73f3

github-actions bot removed the qt Relates to qt label Jun 25, 2021

tlambert03 approved these changes Jun 25, 2021

View reviewed changes

sofroniewn merged commit b34c8e0 into napari:master Jun 26, 2021

GenevieveBuckley mentioned this pull request Mar 27, 2024

numpy version 2.0 compatibility #6776

Merged

Uh oh!

Conversation

JoOkuma commented Jun 23, 2021

Description

Type of change

References

How has this been tested?

Final checklist:

Note

Uh oh!

tlambert03 left a comment

Choose a reason for hiding this comment

Uh oh!

tlambert03 Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

JoOkuma Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

tlambert03 Jun 23, 2021

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoOkuma commented Jun 23, 2021

Uh oh!

JoOkuma commented Jun 24, 2021

Uh oh!

tlambert03 commented Jun 24, 2021

Uh oh!

sofroniewn commented Jun 24, 2021

Uh oh!

tlambert03 commented Jun 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sofroniewn commented Jun 24, 2021

Uh oh!

tlambert03 commented Jun 25, 2021

Uh oh!

JoOkuma commented Jun 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlambert03 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jun 23, 2021 •

edited

Loading

tlambert03 commented Jun 24, 2021 •

edited

Loading

JoOkuma commented Jun 25, 2021 •

edited

Loading