Skip to content

More removal of np.all(... == ...)#6238

Merged
andy-sweet merged 3 commits intonapari:mainfrom
Carreau:less-np-all
Sep 20, 2023
Merged

More removal of np.all(... == ...)#6238
andy-sweet merged 3 commits intonapari:mainfrom
Carreau:less-np-all

Conversation

@Carreau
Copy link
Copy Markdown
Contributor

@Carreau Carreau commented Sep 19, 2023

See #6126

This is in general stricter, more correct and in some case faster.

@github-actions github-actions bot added the tests Something related to our tests label Sep 19, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 19, 2023

Codecov Report

Merging #6238 (f48f79e) into main (eaff277) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #6238   +/-   ##
=======================================
  Coverage   91.60%   91.60%           
=======================================
  Files         583      583           
  Lines       51364    51364           
=======================================
+ Hits        47051    47054    +3     
+ Misses       4313     4310    -3     
Files Changed Coverage Δ
napari/layers/_tests/test_utils.py 100.00% <100.00%> (ø)
napari/layers/image/_tests/test_image.py 100.00% <100.00%> (ø)
napari/layers/shapes/_tests/test_shapes.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

Copy link
Copy Markdown
Member

@andy-sweet andy-sweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I left a comment on the related issue, but this looks good regardless, as it will fix issues with empty arrays.

Copy link
Copy Markdown
Collaborator

@Czaki Czaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but Andy's suggestion may be a good approach for future ones.

@Czaki Czaki added maintenance PR with maintance changes, ready to merge Last chance for comments! Will be merged in ~24h labels Sep 19, 2023
@Carreau
Copy link
Copy Markdown
Contributor Author

Carreau commented Sep 20, 2023

Looks good, but Andy's suggestion may be a good approach for future ones.

Yes, it should be straitforward once we bump the min version to do bulk replace.

@Carreau Carreau added this to the 0.5.0 milestone Sep 20, 2023
Carreau added a commit to Carreau/napari that referenced this pull request Sep 20, 2023
As discussed in napari#6238 and napari#6126.

This will conflict with the former one once merged but is easy to rebase
and do en-masse anyway.

The error messages are no as-nice with numpy below 1.24, but still
enough to see the differences.
@andy-sweet andy-sweet modified the milestone: 0.5.0 Sep 20, 2023
@andy-sweet andy-sweet added maintenance PR with maintance changes, and removed maintenance PR with maintance changes, labels Sep 20, 2023
@andy-sweet andy-sweet merged commit ee862bb into napari:main Sep 20, 2023
@andy-sweet andy-sweet removed the ready to merge Last chance for comments! Will be merged in ~24h label Sep 20, 2023
kne42 added a commit to kne42/napari that referenced this pull request Sep 21, 2023
* main:
  Move `test_file_menu.py` to new app model location (napari#6233)
  Add toggle status view menu items (napari#6137)
  More removal of np.all(... == ...) (napari#6238)
  [pre-commit.ci] pre-commit autoupdate (napari#6229)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PR with maintance changes, tests Something related to our tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants