Darren Dale <[EMAIL PROTECTED]> writes:

> You should also make your rc changes before importing pylab. That might be 
> considered a bug, and would take some work to fix.

I think I've fixed the bug (well, mostly see TODO), as well as a few other
minor bugs:

- ``__init__.py``
  * defaultParams value tuple[0] ought to be the parsed value, as
    far as I can see, but the fonts are given as comma-seperated string, which
    will result in texmanager treating individual chars as 'fonts'

- ``texmanager.py``
  * 'Computer Modern Sans serif' rather than 'Computer Modern Sans-serif' as
    on wiki; I haven't changed it because I don't know which spelling is
    desired -- I'd personally prefer two, because it's consistent with the
    most common spelling of 'sans-serif' and the spelling elsewhere in MPL
  * condensed `TexManger.__init__` and fixed the undeclare var bug in the
    warning; also added check that rcParams['font.*'] is not a string (see
    above)
  * `TexManger.get_font_config` now checks that font configuration is in
    synch with rcParams and if not re-initialized `self` to recompute it
  * changed ``self._font_config -> self.get_font_config()`` and added it to the 
key
    generation for TexManger.get_rgba -- so now every change the relevant
    rcParams font entries will now recompute or select the *correct* cached
    value (Note I think for get_rgba ._font_config would suffice, but since it
    looks like it needs to be modified anyway and .get_font_config is safer I
    opted for the latter).
  * added a few consistency checks

This is the first time I've really looked at (and hacked) the MPL code, so my
solution might have issues, I'm certainly not aware of the big picture.
However I'm pretty sure that parts of the patch should be useful and that I've
correctly identified some small bugs.

TODO
----

A) the wiki page still needs to be corrected; I'm happy to do it, but it would
   be useful to know:
   1. what exactly I should change to ``rcParams`` (all ``rc`` calls?)
   2. how the 'Computer Modern Sans'-'/' 'serif' thing will be resolved
   3. if this patch is likely to be accepted in some form (because then I'll
      also mention that the need to import pylab afterwards will go away in
      svn/future versions)

B) TexManger.get_rgba needs to deal with font-properties other than size to do
   its job properly (i.e. return the *correct* cached text); that means that 2
   places in backend_agg.py will also need changs since it uses insufficient
   info to find the right cached text (no properites). How to do this involves
   making design decisions that I think one of the developers has to take, so
   I didn't do anything about it. I got the impression that just passing
   ``prop.font_family`` together with ``size`` might handle most cases, but I
   have no idea how latex-font-properties are really meant to be handled.

In addition to the patch, I've appended some file to test and demonstrate the
new (hopefully better); behavior (just %run) -- the patch currently is full of
print-DEBUG statements, I've left them in because it makes easier to verify
what's going on and B) still ought to be fixed; they're easy to take remove
(grep DEBUG), but if it's an issue I'm happy to supply the patch without them.

hope this is useful,

'as

from matplotlib.pylab import *
from numpy import *
rc('text', usetex=True)
rc('text', **{'latex.preamble': [r'\usepackage{bm}']})
#figure()
print "Testing that font changes result in cache-invalidation"
print "------------------------------------------------------"
for font_conf in ({'family':'sans-serif','sans-serif':['Computer Modern Sans serif']},
                  {'family':'sans-serif','sans-serif':['avant garde']},
                  {'family':'sans-serif','sans-serif':['Helvetica']},


                  dict(family='serif',serif=['Palatino']),
                  dict(family='serif',serif=['Times']),
                  dict(family='serif',serif=['Bookman']),
                  dict(family='serif',serif=['Computer Modern Roman']),                  
                  ): 
    clf()    
    rc('font',**font_conf)
    title(font_conf)
    text(0,0, r'I will $\bm{g\epsilon\tau}$ RECOMPUTED correctly!',
         fontsize=25,verticalalignment='center',horizontalalignment='center')
    axis([-1, 1, -1, 1])
    show(); raw_input('Press Return')

print "Testing that illegal font-specs get detected"
print "--------------------------------------------"
raw_input('RETURN')
clf()
# should throw
try:
    rc('text', **{'latex.preamble': r'\usepackage{bm}'})
    text(0,0, 'I will get RECOMPUTED correctly!',
          fontsize=25,verticalalignment='center',horizontalalignment='center')
    show();
except AssertionError, msg:
    print "-- SUCCESS: DETECTED CORRUPTION --", msg
    rc('text', **{'latex.preamble': [r'\usepackage{bm}']})
else:
    raise "FAILED"
try:
    rc('font',**{'family':'sans-serif','sans-serif':'Helvetica'})
    text(0,0, 'I will get RECOMPUTED correctly!',
          fontsize=25,verticalalignment='center',horizontalalignment='center')
    show();
except AssertionError, msg:
    print "-- SUCCESS: DETECTED CORRUPTION --", msg
else:
    raise "FAILED"

    
*** __init__.py.ORIG	2007-05-17 18:17:28.000000000 +0200
--- __init__.py	2007-05-18 12:33:19.000000000 +0200
***************
*** 746,750 ****
      'lines.marker'       : ['None', str],     # black
      'lines.markeredgewidth'       : [0.5, validate_float],
!     'lines.markersize'  : [6, validate_float],       # markersize, in points
      'lines.antialiased' : [True, validate_bool],     # antialised (no jaggies)
      'lines.dash_joinstyle' : ['miter', validate_joinstyle],
--- 746,750 ----
      'lines.marker'       : ['None', str],     # black
      'lines.markeredgewidth'       : [0.5, validate_float],
!     'lines.markersize'  : [6.0, validate_float],       # markersize, in points
      'lines.antialiased' : [True, validate_bool],     # antialised (no jaggies)
      'lines.dash_joinstyle' : ['miter', validate_joinstyle],
***************
*** 767,775 ****
      'font.weight'       : ['normal', str],           #
      'font.size'         : [12.0, validate_float], #
!     'font.serif'        : ['Bitstream Vera Serif, New Century Schoolbook, Century Schoolbook L, Utopia, ITC Bookman, Bookman, Nimbus Roman No9 L, Times New Roman, Times, Palatino, Charter, serif', validate_comma_sep_str],
!     'font.sans-serif'   : ['Bitstream Vera Sans, Lucida Grande, Verdana, Geneva, Lucid, Arial, Helvetica, Avant Garde, sans-serif', validate_comma_sep_str],
!     'font.cursive'      : ['Apple Chancery, Textile, Zapf Chancery, Sand, cursive', validate_comma_sep_str],
!     'font.fantasy'      : ['Comic Sans MS, Chicago, Charcoal, Impact, Western, fantasy', validate_comma_sep_str],
!     'font.monospace'    : ['Bitstream Vera Sans Mono, Andale Mono, Nimbus Mono L, Courier New, Courier, Fixed, Terminal, monospace', validate_comma_sep_str],
  
      # text props
--- 767,775 ----
      'font.weight'       : ['normal', str],           #
      'font.size'         : [12.0, validate_float], #
!     'font.serif'        : ['Bitstream Vera Serif, New Century Schoolbook, Century Schoolbook L, Utopia, ITC Bookman, Bookman, Nimbus Roman No9 L, Times New Roman, Times, Palatino, Charter, serif'.split(', '), validate_comma_sep_str],
!     'font.sans-serif'   : ['Bitstream Vera Sans, Lucida Grande, Verdana, Geneva, Lucid, Arial, Helvetica, Avant Garde, sans-serif'.split(', '), validate_comma_sep_str],
!     'font.cursive'      : ['Apple Chancery, Textile, Zapf Chancery, Sand, cursive'.split(', '), validate_comma_sep_str],
!     'font.fantasy'      : ['Comic Sans MS, Chicago, Charcoal, Impact, Western, fantasy'.split(', '), validate_comma_sep_str],
!     'font.monospace'    : ['Bitstream Vera Sans Mono, Andale Mono, Nimbus Mono L, Courier New, Courier, Fixed, Terminal, monospace'.split(', '), validate_comma_sep_str],
  
      # text props
***************
*** 827,842 ****
  
      # tick properties
!     'xtick.major.size'   : [4, validate_float],      # major xtick size in points
!     'xtick.minor.size'   : [2, validate_float],      # minor xtick size in points
!     'xtick.major.pad'    : [4, validate_float],      # distance to label in points
!     'xtick.minor.pad'    : [4, validate_float],      # distance to label in points
      'xtick.color'        : ['k', validate_color],    # color of the xtick labels
      'xtick.labelsize'    : [12, validate_fontsize], # fontsize of the xtick labels
      'xtick.direction'    : ['in', str],            # direction of xticks
  
!     'ytick.major.size'   : [4, validate_float],      # major ytick size in points
!     'ytick.minor.size'   : [2, validate_float],      # minor ytick size in points
!     'ytick.major.pad'    : [4, validate_float],      # distance to label in points
!     'ytick.minor.pad'    : [4, validate_float],      # distance to label in points
      'ytick.color'        : ['k', validate_color],    # color of the ytick labels
      'ytick.labelsize'    : [12, validate_fontsize], # fontsize of the ytick labels
--- 827,842 ----
  
      # tick properties
!     'xtick.major.size'   : [4.0, validate_float],      # major xtick size in points
!     'xtick.minor.size'   : [2.0, validate_float],      # minor xtick size in points
!     'xtick.major.pad'    : [4.0, validate_float],      # distance to label in points
!     'xtick.minor.pad'    : [4.0, validate_float],      # distance to label in points
      'xtick.color'        : ['k', validate_color],    # color of the xtick labels
      'xtick.labelsize'    : [12, validate_fontsize], # fontsize of the xtick labels
      'xtick.direction'    : ['in', str],            # direction of xticks
  
!     'ytick.major.size'   : [4.0, validate_float],      # major ytick size in points
!     'ytick.minor.size'   : [2.0, validate_float],      # minor ytick size in points
!     'ytick.major.pad'    : [4.0, validate_float],      # distance to label in points
!     'ytick.minor.pad'    : [4.0, validate_float],      # distance to label in points
      'ytick.color'        : ['k', validate_color],    # color of the ytick labels
      'ytick.labelsize'    : [12, validate_fontsize], # fontsize of the ytick labels
***************
*** 850,855 ****
      # figure props
      # figure size in inches: width by height
!     'figure.figsize'    : [ (8,6), validate_nseq_float(2)],
!     'figure.dpi'        : [ 80, validate_float],   # DPI
      'figure.facecolor'  : [ '0.75', validate_color], # facecolor; scalar gray
      'figure.edgecolor'  : [ 'w', validate_color],  # edgecolor; white
--- 850,855 ----
      # figure props
      # figure size in inches: width by height
!     'figure.figsize'    : [ (8.0,6.0), validate_nseq_float(2)],
!     'figure.dpi'        : [ 80.0, validate_float],   # DPI
      'figure.facecolor'  : [ '0.75', validate_color], # facecolor; scalar gray
      'figure.edgecolor'  : [ 'w', validate_color],  # edgecolor; white
***************
*** 863,867 ****
  
  
!     'savefig.dpi'       : [ 100, validate_float],   # DPI
      'savefig.facecolor' : [ 'w', validate_color],  # facecolor; white
      'savefig.edgecolor' : [ 'w', validate_color],  # edgecolor; white
--- 863,867 ----
  
  
!     'savefig.dpi'       : [ 100.0, validate_float],   # DPI
      'savefig.facecolor' : [ 'w', validate_color],  # facecolor; white
      'savefig.edgecolor' : [ 'w', validate_color],  # edgecolor; white
***************
*** 991,995 ****
  def rc_params(fail_on_error=False):
      'Return the default params updated from the values in the rc file'
- 
      fname = matplotlib_fname()
      if not os.path.exists(fname):
--- 991,994 ----
*** texmanager.py.ORIG	2007-05-17 18:17:28.000000000 +0200
--- texmanager.py	2007-05-18 17:53:07.000000000 +0200
***************
*** 33,38 ****
  
  """
! 
! import glob, md5, os, shutil, sys, warnings
  from tempfile import gettempdir
  from matplotlib import get_configdir, get_home, get_data_path, \
--- 33,37 ----
  
  """
! import glob, md5, os, shutil, sys, warnings, copy
  from tempfile import gettempdir
  from matplotlib import get_configdir, get_home, get_data_path, \
***************
*** 47,51 ****
  else: cmd_split = ';'
  
- 
  def get_dvipng_version():
      stdin, stdout = os.popen4('dvipng -version')
--- 46,49 ----
***************
*** 92,96 ****
      cursive = ('pzc', r'\usepackage{chancery}')
      font_family = 'serif'
!     
      font_info = {'new century schoolbook': ('pnc', r'\renewcommand{\rmdefault}{pnc}'),
                  'bookman': ('pbk', r'\renewcommand{\rmdefault}{pbk}'),
--- 90,94 ----
      cursive = ('pzc', r'\usepackage{chancery}')
      font_family = 'serif'
!     font_families  = ('serif', 'sans-serif', 'cursive', 'monospace')    
      font_info = {'new century schoolbook': ('pnc', r'\renewcommand{\rmdefault}{pnc}'),
                  'bookman': ('pbk', r'\renewcommand{\rmdefault}{pbk}'),
***************
*** 108,155 ****
                  'computer modern sans serif': ('cmss', ''),
                  'computer modern typewriter': ('cmtt', '')}
! 
      def __init__(self):
!         
          if not os.path.isdir(self.texcache):
              os.mkdir(self.texcache)
!         if rcParams['font.family'].lower() in ('serif', 'sans-serif', 'cursive', 'monospace'):
!             self.font_family = rcParams['font.family'].lower()
          else:
              warnings.warn('The %s font family is not compatible with LaTeX. serif will be used by default.' % ff)
              self.font_family = 'serif'
!         self._fontconfig = self.font_family
!         for font in rcParams['font.serif']:
!             try:
!                 self.serif = self.font_info[font.lower()]
!             except KeyError:
!                 continue
!             else:
!                 break
!         self._fontconfig += self.serif[0]
!         for font in rcParams['font.sans-serif']:
!             try:
!                 self.sans_serif = self.font_info[font.lower()]
!             except KeyError:
!                 continue
              else:
!                 break
!         self._fontconfig += self.sans_serif[0]
!         for font in rcParams['font.monospace']:
!             try:
!                 self.monospace = self.font_info[font.lower()]
!             except KeyError:
!                 continue
!             else:
!                 break                
!         self._fontconfig += self.monospace[0]
!         for font in rcParams['font.cursive']:
!             try:
!                 self.cursive = self.font_info[font.lower()]
!             except KeyError:
!                 continue
!             else:
!                 break                
!         self._fontconfig += self.cursive[0]
!         
          # The following packages and commands need to be included in the latex 
          # file's preamble:
--- 106,135 ----
                  'computer modern sans serif': ('cmss', ''),
                  'computer modern typewriter': ('cmtt', '')}
!     _rc_cache = None # XXX checking text.latex.preamble might be overkill OTOH it's typically cheap
!     _rc_cache_keys = ('text.latex.preamble',)+tuple('font.' + n for n in ('family',) + font_families)
      def __init__(self):
!         global CUR; CUR = self #DEBUG
          if not os.path.isdir(self.texcache):
              os.mkdir(self.texcache)
!         ff = rcParams['font.family'].lower()
!         if ff in self.font_families:
!             self.font_family = ff
          else:
              warnings.warn('The %s font family is not compatible with LaTeX. serif will be used by default.' % ff)
              self.font_family = 'serif'
!         fontconfig = [self.font_family]
!         for font_family, font_family_attr in ((ff, ff.replace('-','_')) for ff in self.font_families):
!             assert not isinstance(rcParams['font.' + font_family], basestring), '\
! BUG, rcParams[%s] treated as list but is comma-sep string: %s' % (
! 'font.' + font_family, rcParams['font.' + font_family])
!             for font in rcParams['font.' + font_family]:
!                 print '####DEBUG family: %s font: %s info: %s' % (font_family, font, self.font_info.get(font.lower()))
!                 if font.lower() in self.font_info:
!                     setattr(self, font_family_attr, self.font_info[font.lower()])
!                     break
              else:
!                 print >> sys.stderr, "Don't know how to use font %s in LaTeX" % font
!             fontconfig.append(getattr(self, font_family_attr)[0])
!         self._fontconfig = "".join(fontconfig)
          # The following packages and commands need to be included in the latex 
          # file's preamble:
***************
*** 160,172 ****
          cmd = '\n'.join(cmd)
          self._font_preamble = '\n'.join([r'\usepackage{type1cm}',
!                              cmd,
!                              r'\usepackage{textcomp}'])
!         
      def get_basefile(self, tex, fontsize, dpi=None):
!         s = tex + self._fontconfig + ('%f'%fontsize) + self.get_custom_preamble()
!         if dpi: s += ('%s'%dpi)
          return os.path.join(self.texcache, md5.md5(s).hexdigest())
          
      def get_font_config(self):
          return self._fontconfig
          
--- 140,163 ----
          cmd = '\n'.join(cmd)
          self._font_preamble = '\n'.join([r'\usepackage{type1cm}',
!                                          cmd,
!                                          r'\usepackage{textcomp}'])
      def get_basefile(self, tex, fontsize, dpi=None):
!         s = "".join([tex, self.get_font_config(), ('%f'%fontsize), self.get_custom_preamble(), str(dpi or '')])
          return os.path.join(self.texcache, md5.md5(s).hexdigest())
          
      def get_font_config(self):
+         "Reinitializes self if rcParams self depends on have changed."
+         if self._rc_cache is None:
+             self._rc_cache = dict((k,None) for k in self._rc_cache_keys)
+         changed = [par for par in self._rc_cache_keys if rcParams[par] != self._rc_cache[par]]
+         if changed:
+             print '###DEBUG following keys changed:', changed
+             for k in changed:
+                 print '###DEBUG %-20s: %-10s -> %-10s' % (k, self._rc_cache[k], rcParams[k])
+                 # XXX deepcopy is likely not necessary, but feels more future-proof
+                 self._rc_cache[k] = copy.deepcopy(rcParams[k])
+             print '###DEBUG ABOUT TO RE-INIT\n     old fontconfig:', self._fontconfig
+             self.__init__()
+         print '###DEBUG fontconfig:', self._fontconfig
          return self._fontconfig
          
***************
*** 175,179 ****
      
      def get_custom_preamble(self):
!         return '\n'.join(rcParams['text.latex.preamble'])
          
      def get_shell_cmd(self, *args):
--- 166,173 ----
      
      def get_custom_preamble(self):
!         p=rcParams['text.latex.preamble']
!         assert not isinstance(p, basestring), \
!                "latex.preamble must be a list of strings, not a string!:%r" % p
!         return '\n'.join(p)
          
      def get_shell_cmd(self, *args):
***************
*** 208,211 ****
--- 202,206 ----
  \end{document}
  """ % (self._font_preamble, custom_preamble, fontsize, fontsize*1.25, tex)
+         print "###DEBUG {%s}", s
          fh.write(s)
          fh.close()
***************
*** 310,314 ****
          # When you set dvipng -bg Transparent, it actually makes the
          # alpha channel 1 and does the background compositing and
!         # antialiasing itself and puts the blended data in the rgb
          # channels.  So what we do is extract the alpha information
          # from the red channel, which is a blend of the default dvipng
--- 305,309 ----
          # When you set dvipng -bg Transparent, it actually makes the
          # alpha channel 1 and does the background compositing and
!         # antialiasing itself and puts the bolended data in the rgb
          # channels.  So what we do is extract the alpha information
          # from the red channel, which is a blend of the default dvipng
***************
*** 328,335 ****
          if not dpi: dpi = rcParams['savefig.dpi']
          r,g,b = rgb
-         key = tex, fontsize, dpi, tuple(rgb)
-         Z = self.arrayd.get(key)
  
          if Z is None:
              # force=True to skip cacheing while debugging
              pngfile = self.make_png(tex, fontsize, dpi, force=False)
--- 323,332 ----
          if not dpi: dpi = rcParams['savefig.dpi']
          r,g,b = rgb
  
+         key = tex, self.get_font_config(), fontsize, dpi, tuple(rgb)
+         Z = self.arrayd.get(key)
+         print '''###DEBUG looking for Z...''', key
          if Z is None:
+             print '''###DEBUG  ... no right Z, computing'''
              # force=True to skip cacheing while debugging
              pngfile = self.make_png(tex, fontsize, dpi, force=False)

p.s.

If I may make another suggestion (I don't have time to volunteer for
implementing them in the forseeable future, but I might eventually if there's
interest): Maybe the way configuration is handled could be improved:

1. It seems to me that letting users manipulate some raw dictionary
   (`rcParams`) in order to customize behavior is bad.
   
   Just now, on my first dig into matplotlib I've found a number of bugs that
   result from bad state in rcParams. As a user this has also caused me some
   wasted time, because typos and other illegal param choices are not caught
   early (but are relativley likely, because matplotlib has many and complex
   options).

   Using some dedicated class-instance instead of a dict would allow for:
   - automatic conistency checks on setting
   - possibly even change propagation (likely faster and certainly less error
     prone if done right: I'm not sure how many places rcParams dependent
     results apart from the tex-stuff are cached and should be recomputed when
     certain params change, but obviously an push/'event'-based solutin could
     be faster than pull/polling)
   - possibly better performance, because e.g. using immutable values would
     allow identity based checks to see if a rcParam changed and obviate the
     need to copy; 
   - convenient doc lookup and searching an display of available alternatives
     (e.g. font-families etc)
   - better factoring of the matplotlibrc parsing and validation see below:


2. It also seems to me that having some custom config file format rather than
   just (a literal-only subset of) python is suboptimal. Why make people learn
   another and more limited syntax when python literals are already familiar
   and both more powerful and easier to use (e.g. the latex-preamble setting
   can't express multiple package options, because param values can't contain
   ',')? It would also get rid of the IMO undesirable coupling of
   string-parsing and type-checking that the validate_* functions do at the
   moment; really setting an rcParam should also do the latter but not the
   former (and guaranteed validation on setting would also avoid the need to
   sprinkle random checks on reading through the code).

   IMO it would be much nicer if there was some more declarative way to
   specify options, check their validity on setting and assoicate docs with
   them (sort of like e.g. emcs-custom vars).
  
   Valid files in the old-syntax could be translated automatically quite
   easily, so I don't think it's a compatiblity issue.

cheers,

'as

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Matplotlib-users mailing list
Matplotlib-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matplotlib-users

Reply via email to