Skip to content

Fedora build patches #9304

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

Merged
merged 5 commits into from
Oct 23, 2017
Merged

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 7, 2017

PR Summary

Patches needed to build & test on Fedora on multiple arches, with the exception of the deprecation warning capture (which is just nice to have because of the pytest bug). Of course, this does not include the tolerance bump required to build against FreeType 2.7.1 or 2.8.

This works on x86_64 and arm. I need to go through the results of aarch64/ppc64/ppc64le as well.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • [-] New features are documented, with examples if plot related
  • [-] Documentation is sphinx and numpydoc compliant
  • [-] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [-] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

These are either deprecations, or checks for old, but probably
incorrect, behaviour.
For some reason, NaN gets converted to 0 as an integer instead of
INT_MIN like on x86.

Fixes matplotlib#6538.
@QuLogic QuLogic added this to the 2.1.1 (next bug fix release) milestone Oct 7, 2017
@tacaswell
Copy link
Member

@QuLogic are you planning to add more to this?

@QuLogic
Copy link
Member Author

QuLogic commented Oct 10, 2017

I was really hoping to figure out the ppc64 barb issue over the weekend, but as I'm not at a suitable computer at the moment, and have other things to do, I guess that won't be possible.

I believe the remaining build issues are simply due to FreeType 2.7+ and small precision things which we can probably ignore for the time being. Feel free to merge this whenever it's approved.

@QuLogic QuLogic changed the title WIP: Fedora build patches Fedora build patches Oct 10, 2017
@@ -603,7 +603,8 @@ def test_load_from_url():

@image_comparison(baseline_images=['log_scale_image'],
remove_text=True)
def test_log_scale_image():
# The recwarn fixture captures a warning in image_comparison.
def test_log_scale_image(recwarn):
Copy link
Member

Choose a reason for hiding this comment

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

I think this is throwing a warning that we should try and fix instead of ignoring it. Presumably ax.imshow sets axis=equal behind the scenes, which causes ax.set_yscale('log') to throw a warning. If this is all valid code (as I think it is) I don't think it should throw a warning.

Copy link
Member Author

Choose a reason for hiding this comment

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

But how could we catch it? Setting equal aspect ratio and setting log scaling are independent, and I think there's no link saying that equal aspect ratio was requested by imshow.

Copy link
Member

Choose a reason for hiding this comment

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

In principle we could set the aspect to 'auto' after calling imshow but I would expect that to change the output. I am 👍 on just catching this warning.

@QuLogic
Copy link
Member Author

QuLogic commented Oct 15, 2017

So looking at the C standard 6.3.1.4 paragraph 1:

When a finite value of real floating type is converted to an integer type other than _Bool, the fractional part is discarded (i.e., the value is truncated toward zero). If the value of the integral part cannot be represented by the integer type, the behavior is undefined.

I'm not sure if NumPy just defers to C casts there, but I guess if it does, that sort of explains why the cast in AxesImage.get_cursor_data fails only on arm.

@anntzer
Copy link
Contributor

anntzer commented Oct 21, 2017

Probably worth raising this with numpy? Obviously unlikely to help now, but I think it would make sense for the numpy error system to report such overflows/UB...

@QuLogic
Copy link
Member Author

QuLogic commented Oct 22, 2017

There appear to be a few upstream reports about this; numpy/numpy#8325 seems farthest along and they seem to have come to the same root cause.

@dopplershift dopplershift merged commit 10ef9e1 into matplotlib:v2.1.x Oct 23, 2017
@QuLogic QuLogic deleted the fedora-build-patches branch October 23, 2017 20:09
@anntzer
Copy link
Contributor

anntzer commented Oct 24, 2017

Wait, this was not merged into master?
@meeseeksdev backport to master

lumberbot-app bot pushed a commit that referenced this pull request Oct 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants