Skip to content
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

Path.contains_points() returns a uint8 array instead of a bool array #3824

Merged
merged 1 commit into from Nov 21, 2014

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Nov 21, 2014

According to the docs, matplotlib.path.Path.contains_points() ought to return a boolean array, but it currently returns a uint8 array. It looks like this API change was introduced in ba40160. It caught me out when I tried to use the result of contains_points() as a boolean index to another array.

@jenshnielsen
Copy link
Member

I can confirm this. It works as intended in 1.4.2 so it seems likely to be a result of the c++ refactor.

@jenshnielsen jenshnielsen added this to the v1.5.x milestone Nov 21, 2014
mdboom added a commit to mdboom/matplotlib that referenced this pull request Nov 21, 2014
@mdboom
Copy link
Member

mdboom commented Nov 21, 2014

Fix attached.

}
}
inside_flag[i] |= subpath_flag[i];
if (inside_flag[i] == 0) {
inside_flag[i] = inside_flag[i] || subpath_flag[i];
Copy link
Member

Choose a reason for hiding this comment

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

Was this one necessary? (Don't change it, I'm just curious)

Copy link
Member Author

Choose a reason for hiding this comment

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

In C++ the | operator is not valid on bool, and ||= does not exist. So AFAICT, you have to do something like this.

pelson added a commit that referenced this pull request Nov 21, 2014
Path.contains_points() returns a uint8 array instead of a bool array
@pelson pelson merged commit 443afd3 into matplotlib:master Nov 21, 2014
@pelson
Copy link
Member

pelson commented Nov 21, 2014

👍 - swift @mdboom.

Thanks for reporting @alimuldal. It's not often a bug goes from reported to merged in < 2 hours 😉

@alimuldal
Copy link
Contributor Author

Awesome, thanks guys

@mdboom mdboom deleted the contains-bool branch March 3, 2015 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants