Menu
▾
▴
matplotlib-users
|
From: Nicolas M. <nic...@la...> - 2013-07-08 14:57:36
|
Hi,
In matplotlib 1.2.1, the get_name function is not garding against none
self (unlike other functions); Unfortunately it seems I have a workload
that makes matplotlib call get_name with None (wasn't the case in 1.2.0).
I couldn't isolate the exact trigger, when I reduce the volume of data
processed the problem goes away so I have to simple shareable reproducer.
Anyway, the following patch makes it all work for me, could it (or
something similar) be merged?
diff -uNr matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py
matplotlib-1.2.1/lib/matplotlib/font_manager.py
--- matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py 2013-03-26
14:04:37.000000000 +0100
+++ matplotlib-1.2.1/lib/matplotlib/font_manager.py 2013-07-08
14:49:37.791845661 +0200
@@ -721,6 +721,8 @@
Return the name of the font that best matches the font
properties.
"""
+ if self._family is None:
+ return rcParams['font.family']
return ft2font.FT2Font(str(findfont(self))).family_name
def get_style(self):
Regards,
--
Nicolas Mailhot
|
|
From: Martin M. <mmo...@fo...> - 2013-07-08 16:18:18
|
Nicolas Mailhot wrote: > Hi, > > In matplotlib 1.2.1, the get_name function is not garding against none > self (unlike other functions); Unfortunately it seems I have a workload > that makes matplotlib call get_name with None (wasn't the case in 1.2.0). > I couldn't isolate the exact trigger, when I reduce the volume of data > processed the problem goes away so I have to simple shareable reproducer. > > Anyway, the following patch makes it all work for me, could it (or > something similar) be merged? > > diff -uNr matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py > matplotlib-1.2.1/lib/matplotlib/font_manager.py > --- matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py 2013-03-26 > 14:04:37.000000000 +0100 > +++ matplotlib-1.2.1/lib/matplotlib/font_manager.py 2013-07-08 > 14:49:37.791845661 +0200 > @@ -721,6 +721,8 @@ > Return the name of the font that best matches the font > properties. > """ > + if self._family is None: > + return rcParams['font.family'] > return ft2font.FT2Font(str(findfont(self))).family_name > > def get_style(self): > > Regards, > Could you instead just test for "if not self._family"? Tests for equality are more expensive (that means self._family == 0 or self._family == False will also trigger your return). |
|
From: Nicolas M. <nic...@la...> - 2013-07-09 13:15:16
|
Le Lun 8 juillet 2013 18:18, Martin Mokrejs a écrit : > Nicolas Mailhot wrote: >> Hi, >> >> In matplotlib 1.2.1, the get_name function is not garding against none >> self (unlike other functions); Unfortunately it seems I have a workload >> that makes matplotlib call get_name with None (wasn't the case in >> 1.2.0). >> I couldn't isolate the exact trigger, when I reduce the volume of data >> processed the problem goes away so I have to simple shareable >> reproducer. >> >> Anyway, the following patch makes it all work for me, could it (or >> something similar) be merged? >> >> diff -uNr matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py >> matplotlib-1.2.1/lib/matplotlib/font_manager.py >> --- matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py >> 2013-03-26 >> 14:04:37.000000000 +0100 >> +++ matplotlib-1.2.1/lib/matplotlib/font_manager.py 2013-07-08 >> 14:49:37.791845661 +0200 >> @@ -721,6 +721,8 @@ >> Return the name of the font that best matches the font >> properties. >> """ >> + if self._family is None: >> + return rcParams['font.family'] >> return ft2font.FT2Font(str(findfont(self))).family_name >> >> def get_style(self): >> >> Regards, >> > > Could you instead just test for "if not self._family"? Tests for equality > are more expensive (that means self._family == 0 or self._family == False > will also trigger your return). I can test if it works. However, all the other tests in that file are already of the if Foo is None form, and I didn't want to change the coding style Regards, -- Nicolas Mailhot |
|
From: Nicolas M. <nic...@la...> - 2013-07-09 14:30:18
|
Le Mar 9 juillet 2013 15:14, Nicolas Mailhot a écrit :
>
> Le Lun 8 juillet 2013 18:18, Martin Mokrejs a écrit :
>> Could you instead just test for "if not self._family"? Tests for
>> equality
>> are more expensive (that means self._family == 0 or self._family ==
>> False
>> will also trigger your return).
>
> I can test if it works. However, all the other tests in that file are
> already of the if Foo is None form, and I didn't want to change the coding
> style
And I can confirm the following patch also fixes my workload
diff -uNr matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py
matplotlib-1.2.1/lib/matplotlib/font_manager.py
--- matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py 2013-03-26
14:04:37.000000000 +0100
+++ matplotlib-1.2.1/lib/matplotlib/font_manager.py 2013-07-08
14:49:37.791845661 +0200
@@ -721,6 +721,8 @@
Return the name of the font that best matches the font
properties.
"""
+ if not self._family:
+ return rcParams['font.family'][0]
return ft2font.FT2Font(str(findfont(self))).family_name
def get_style(self):
Now could something similar be merged?
Regards,
--
Nicolas Mailhot
|
|
From: Martin M. <mmo...@fo...> - 2013-07-09 15:03:22
|
Hi Nicolas, Nicolas Mailhot wrote: > > Le Mar 9 juillet 2013 15:14, Nicolas Mailhot a écrit : >> >> Le Lun 8 juillet 2013 18:18, Martin Mokrejs a écrit : > >>> Could you instead just test for "if not self._family"? Tests for >>> equality >>> are more expensive (that means self._family == 0 or self._family == >>> False >>> will also trigger your return). >> >> I can test if it works. However, all the other tests in that file are >> already of the if Foo is None form, and I didn't want to change the coding >> style I think it is a bad style then. If in some places a '' or False is needed to discriminate from None then the code should be changed. > > And I can confirm the following patch also fixes my workload > > diff -uNr matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py > matplotlib-1.2.1/lib/matplotlib/font_manager.py > --- matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py 2013-03-26 > 14:04:37.000000000 +0100 > +++ matplotlib-1.2.1/lib/matplotlib/font_manager.py 2013-07-08 > 14:49:37.791845661 +0200 > @@ -721,6 +721,8 @@ > Return the name of the font that best matches the font > properties. > """ > + if not self._family: > + return rcParams['font.family'][0] > return ft2font.FT2Font(str(findfont(self))).family_name > > def get_style(self): > > Now could something similar be merged? Thank you for your efforts on this. I am just a user like you so we have to wait for an answer of the maintainers. Maybe I should have emphasized that earlier. ;-) But this is good for the community review anyway. ;) Martin |
|
From: Michael D. <md...@st...> - 2013-07-17 13:05:08
|
This patch doesn't make a whole lot of sense to me. get_name should work just fine if self._family is None, and indeed it does in my own testing: ``` from matplotlib import font_manager f = font_manager.FontProperties(None) print f._family print f.get_family() print f.get_name() ``` So I'd much prefer to get to the bottom of the root cause of this problem than patch it unnecessarily in this way. Any idea what that is? Mike On 07/08/2013 10:57 AM, Nicolas Mailhot wrote: > Hi, > > In matplotlib 1.2.1, the get_name function is not garding against none > self (unlike other functions); Unfortunately it seems I have a workload > that makes matplotlib call get_name with None (wasn't the case in 1.2.0). > I couldn't isolate the exact trigger, when I reduce the volume of data > processed the problem goes away so I have to simple shareable reproducer. > > Anyway, the following patch makes it all work for me, could it (or > something similar) be merged? > > diff -uNr matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py > matplotlib-1.2.1/lib/matplotlib/font_manager.py > --- matplotlib-1.2.1.orig/lib/matplotlib/font_manager.py 2013-03-26 > 14:04:37.000000000 +0100 > +++ matplotlib-1.2.1/lib/matplotlib/font_manager.py 2013-07-08 > 14:49:37.791845661 +0200 > @@ -721,6 +721,8 @@ > Return the name of the font that best matches the font > properties. > """ > + if self._family is None: > + return rcParams['font.family'] > return ft2font.FT2Font(str(findfont(self))).family_name > > def get_style(self): > > Regards, > |
|
From: Nicolas M. <nic...@la...> - 2013-07-18 12:13:09
|
Le Mer 17 juillet 2013 15:04, Michael Droettboom a écrit :
> This patch doesn't make a whole lot of sense to me. get_name should
> work just fine if self._family is None, and indeed it does in my own
> testing:
>
> ```
> from matplotlib import font_manager
>
> f = font_manager.FontProperties(None)
> print f._family
> print f.get_family()
> print f.get_name()
> ```
>
> So I'd much prefer to get to the bottom of the root cause of this
> problem than patch it unnecessarily in this way. Any idea what that is?
I haven't the faintest idea. The function seems to be called with a "None"
font, and then it crashes. As it's executed under a Linux system it uses
the fontconfig backend with cairo. The call only occurs when matplotlib is
fed more data than can be isolated in an simple example
Attaching the only patches Fedora/Red Hat applied before the build :
Patch0: %{name}-noagg.patch
Patch1: %{name}-tk.patch
# http://sourceforge.net/mailarchive/message.php?msg_id=30202451
# https://github.com/matplotlib/matplotlib/pull/1666
# https://bugzilla.redhat.com/show_bug.cgi?id=896182
Patch2: %{name}-fontconfig.patch
[…]
# Remove bundled libraries
rm -r agg24 lib/matplotlib/pyparsing_py?.py
# Remove references to bundled libraries
%patch0 -p1 -b .noagg
sed -i -e s/matplotlib\.pyparsing_py./pyparsing/g lib/matplotlib/*.py
# Correct tcl/tk detection
%patch1 -p1 -b .tk
sed -i -e 's|@@libdir@@|%{_libdir}|' setupext.py
# Use fontconfig by default
%patch2 -p1 -b .fontconfig
Regards,
--
Nicolas Mailhot |
|
From: Nicolas M. <nic...@la...> - 2013-07-19 15:20:23
|
Le Jeu 18 juillet 2013 14:12, Nicolas Mailhot a écrit :
> Le Mer 17 juillet 2013 15:04, Michael Droettboom a écrit :
>> This patch doesn't make a whole lot of sense to me. get_name should
>> work just fine if self._family is None, and indeed it does in my own
>> testing:
>>
>> ```
>> from matplotlib import font_manager
>>
>> f = font_manager.FontProperties(None)
>> print f._family
>> print f.get_family()
>> print f.get_name()
>> ```
>>
>> So I'd much prefer to get to the bottom of the root cause of this
>> problem than patch it unnecessarily in this way. Any idea what that is?
>
> I haven't the faintest idea. The function seems to be called with a "None"
> font, and then it crashes. As it's executed under a Linux system it uses
> the fontconfig backend with cairo. The call only occurs when matplotlib is
> fed more data than can be isolated in an simple example
>
> Attaching the only patches Fedora/Red Hat applied before the build :
>
> Patch0: %{name}-noagg.patch
> Patch1: %{name}-tk.patch
> # http://sourceforge.net/mailarchive/message.php?msg_id=30202451
> # https://github.com/matplotlib/matplotlib/pull/1666
> # https://bugzilla.redhat.com/show_bug.cgi?id=896182
> Patch2: %{name}-fontconfig.patch
>
> […]
>
> # Remove bundled libraries
> rm -r agg24 lib/matplotlib/pyparsing_py?.py
>
> # Remove references to bundled libraries
> %patch0 -p1 -b .noagg
> sed -i -e s/matplotlib\.pyparsing_py./pyparsing/g lib/matplotlib/*.py
>
> # Correct tcl/tk detection
> %patch1 -p1 -b .tk
> sed -i -e 's|@@libdir@@|%{_libdir}|' setupext.py
>
> # Use fontconfig by default
> %patch2 -p1 -b .fontconfig
BTW if you have some code to add to the crashing function to identify
what's wrong, I can reproduce the crash at will (though the processing
before crash time is rather long)
Regards,
--
Nicolas Mailhot
|
