fix alltrue

[homosapien-lcy]

Change all the numpy.alltrue to numpy.all

Fixes #991

[corranwebster]

This looks OK - CI is currently broken for Enable because of an issue with ubuntu latest however.

A couple of comments:

• when we know that the argument of all is an array of dtype bool (rather than a list or other iterable) then it might be better to use (...).all() and avoid an import at all. This will be the case in most of the tests, but not in functions like fill_equal where we have no idea what is being passed in.
• it's probably clearer to use np.all instead of all to avoid confusion with the built-in all.

Update: CI is now fixed on the main branch, so you will need to merge main to get tests to pass.

[homosapien-lcy]

This looks OK - CI is currently broken for Enable because of an issue with ubuntu latest however.

A couple of comments:

• when we know that the argument of all is an array of dtype bool (rather than a list or other iterable) then it might be better to use (...).all() and avoid an import at all. This will be the case in most of the tests, but not in functions like fill_equal where we have no idea what is being passed in.
• it's probably clearer to use np.all instead of all to avoid confusion with the built-in all.

Update: CI is now fixed on the main branch, so you will need to merge main to get tests to pass.

Thanks

[homosapien-lcy]

This looks OK - CI is currently broken for Enable because of an issue with ubuntu latest however.

A couple of comments:

• when we know that the argument of all is an array of dtype bool (rather than a list or other iterable) then it might be better to use (...).all() and avoid an import at all. This will be the case in most of the tests, but not in functions like fill_equal where we have no idea what is being passed in.
• it's probably clearer to use np.all instead of all to avoid confusion with the built-in all.

Update: CI is now fixed on the main branch, so you will need to merge main to get tests to pass.

This looks OK - CI is currently broken for Enable because of an issue with ubuntu latest however.

A couple of comments:

• when we know that the argument of all is an array of dtype bool (rather than a list or other iterable) then it might be better to use (...).all() and avoid an import at all. This will be the case in most of the tests, but not in functions like fill_equal where we have no idea what is being passed in.
• it's probably clearer to use np.all instead of all to avoid confusion with the built-in all.

Update: CI is now fixed on the main branch, so you will need to merge main to get tests to pass.

Actually, one thing I would like to discuss about (want to get clarified before making the final change):
In the scenarios you described, if the input is a numpy array, then .all() would do the job.
If the input is other objects, all() (the python builtin all, not the np.all) will generate the right result.
So it seems that there is no need for us to use np.all in any cases?

[corranwebster]

Actually, one thing I would like to discuss about (want to get clarified before making the final change): In the scenarios you described, if the input is a numpy array, then .all() would do the job. If the input is other objects, all() (the python builtin all, not the np.all) will generate the right result. So it seems that there is no need for us to use np.all in any cases?

In the second case you still want to use np.all because I think (without actually checking!) that builtin all won't handle multidimensional numpy arrays correctly, but np.all will. In general if you are likely to get arrays, but might get lists or other iterables, use np.all(); if you aren't using numpy, use builtin all. Edit: and we do have some 2D arrays in this code - there are tests for equality of affine matrices).

However with a bit of a closer look at the actual code, it looks like everything can be done with .all() modulo a couple of tweaks for clarity which I will highlight for you.

[corranwebster]

This whole section could be re-written to be much clearer in intent

Suggested change

	if not all(pattern):
	self.state.line_state.line_dash = NO_DASH
	return
	pattern = asarray(pattern)
	pattern = asarray(pattern)
	if (pattern == 0).any():
	self.state.line_state.line_dash = NO_DASH
	return

[corranwebster]

If you really want to clean things up, all of these self.assertTrue(all(... == ...)) can be replaced with:

from numpy.testing import assert_array_equal
...
assert_array_equal(..., ...)

eg. the above could be simply assert_array_equal(actual, desired)

[corranwebster]

Something seems to have gone wrong with the merge. Normal merge process for main is:

• switch to main branch and git pull to get latest version (there are probably cleverer ways to do it, but this is simple)
• switch back to the working branch and perform git merge main
  • if there are conflicts, resolve them, and git add and git commit the fixes to complete the merge

This will give you a single known-good merge commit on your working branch.

It looks like you may have cherry-picked instead of merging?

It looks like PR can still be merged, but it is making the review more difficult because it is hard to see what is a new change vs. what is the same as the main.

One possibility is that given that there are a lot of changes needed, it may be easier just to put this PR on hold and create a new, clean PR starting from the head of main.

Edit: alternatively I think if you do the steps above (in particular the git pull on the main branch is crucial, as main has been updated again) then I think git will sort everything out and we should be back in a reasonable state for reviewing.

[mdickinson]

Something seems to have gone wrong with the merge. [...]

FWIW, I've enabled the "Always suggest updating pull request branches" setting in this repository. With that in place, you should be able to simply click the "Update branch" button to merge main in (assuming that there aren't any conflicts).

[homosapien-lcy]

Yeah, I think new PR would be easier, I will start a new one

[homosapien-lcy]

I just find that when the builtin all function is applied to a multi dimensional list, it will treat list as individual objects (aka, if the list is empty, treat as False, if not, treat as True). Thus it will return True for multi-D list as long as all sublists in it are not empty.

[homosapien-lcy]

The latest updates are in issue #1015