Skip to content

Implemented issue 4044. Created a Cell subclass, SciCell, to override th... #4245

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

Closed
wants to merge 6 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Implemented issue 4044. Created a Cell subclass, SciCell, to override…
… the default draw function. SciCell draws a polygon (line) instead of the rectangle
  • Loading branch information
anthonycho committed Mar 18, 2015
commit 9eb3204a558afdc1ece1a1173c941a67b3d96db5
55 changes: 51 additions & 4 deletions lib/matplotlib/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

from . import artist
from .artist import Artist, allow_rasterization
from .patches import Rectangle
from .patches import Rectangle, Polygon
from .cbook import is_string_like
from matplotlib import docstring
from .text import Text
Expand Down Expand Up @@ -145,6 +145,31 @@ def set_text_props(self, **kwargs):
'update the text properties with kwargs'
self._text.update(kwargs)

class SciCell(Cell):

@allow_rasterization
def draw(self, renderer):
if not self.get_visible():
return

bbox = Rectangle.get_bbox(self)
x, y, w, h = bbox.bounds

topLineVertices = [[x, y],[x + w, y]]
botLineVertices = [[x, y + h],[x + w, y + h]]

topLine = Polygon(topLineVertices)
botLine = Polygon(botLineVertices)

topLine.update_from(self)
botLine.update_from(self)

topLine.draw(renderer)
botLine.draw(renderer)

# position the text
self._set_text_position(renderer)
self._text.draw(renderer)

class Table(Artist):
"""
Expand Down Expand Up @@ -211,12 +236,17 @@ def __init__(self, ax, loc=None, bbox=None, **kwargs):
self.set_clip_on(False)

self._cachedRenderer = None
self._cellType = 'default'
Copy link
Member

Choose a reason for hiding this comment

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

underscores instead of camel case please. Yes, the code base is mixed, but lets not make it worse with new additions.

Copy link
Member

Choose a reason for hiding this comment

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

actually, scratch that, better to match internally in this class.


def add_cell(self, row, col, *args, **kwargs):
""" Add a cell to the table. """
xy = (0, 0)

cell = Cell(xy, *args, **kwargs)
if self._cellType == 'default':
Copy link
Member

Choose a reason for hiding this comment

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

I would use a class-level dictionary to do this look up

cell = Cell(xy, *args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

So this can be just cell = self.AVAILABILECELLTYPES[self._cell_type]

Copy link
Member

Choose a reason for hiding this comment

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

and if you are using a property, you should use it here instead of touching the internals.

else:
cell = SciCell(xy, *args, **kwargs)

cell.set_figure(self.figure)
cell.set_transform(self.get_transform())

Expand Down Expand Up @@ -453,16 +483,27 @@ def get_celld(self):
'return a dict of cells in the table'
return self._cells

def get_cell_type(self):
Copy link
Member

Choose a reason for hiding this comment

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

Can you do this as a property?

return self._cellType

def set_cell_type(self, cellType):
if cellType in ('default', 'scicell'):
self._cellType = cellType

else:
raise ValueError('Unrecognized cell type %s; '
'try default or scicell' % cellType)


def table(ax,
cellText=None, cellColours=None,
cellType=None, cellText=None, cellColours=None,
Copy link
Member

Choose a reason for hiding this comment

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

This has to go last to maintain back-compatibility for anyone who has been using this with positional arguments.

cellLoc='right', colWidths=None,
rowLabels=None, rowColours=None, rowLoc='left',
colLabels=None, colColours=None, colLoc='center',
loc='bottom', bbox=None,
**kwargs):
"""
TABLE(cellText=None, cellColours=None,
TABLE(cellType='default', cellText=None, cellColours=None,
cellLoc='right', colWidths=None,
rowLabels=None, rowColours=None, rowLoc='left',
colLabels=None, colColours=None, colLoc='center',
Expand All @@ -472,6 +513,9 @@ def table(ax,

Thanks to John Gill for providing the class and table.
"""
if cellType is not None:
assert cellType in ('default', 'scicell')
Copy link
Member

Choose a reason for hiding this comment

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

We just (well in the process of #3060 needs one more read through before it is merged) converting all of the asserts into raises. Please raise a ValueError here.

The logic behind this is that if it is really a problem that will cause the code to fail, then it should not be possible to avoid the checks (which you can do with asserts via interpreter flags).


# Check we have some cellText
if cellText is None:
# assume just colours are needed
Expand Down Expand Up @@ -529,6 +573,9 @@ def table(ax,
# Now create the table
table = Table(ax, loc, bbox, **kwargs)
height = table._approx_text_height()

if cellType is not None:
Copy link
Member

Choose a reason for hiding this comment

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

The property should deal with the None internally so you don't need this check.

table.set_cell_type(cellType)

# Add the cells
for row in xrange(rows):
Expand Down