Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
… the default draw function. SciCell draws a polygon (line) instead of the rectangle
| 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': |
There was a problem hiding this comment.
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
This is a great start! I left a bunch of comments, some are style related some are code-design related. I know it may seem a bit intimidating/hostile, but don't be discouraged. I am happy to work with you through the process of getting this PR ready to merge and answer any questions you have. |
| """ | ||
| def __init__(self, *args, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You don't actually need the__init__ here if all you do is call the base class__init__
Hi Thomas, just a quick question, did I implement the class level dictionary correctly? That was the only confusing part for me because I didn't know what the value should be (it would be a list if we didn't need a pairing). Thanks! |
@anthonycho I restarted the failing job. Looks like it was just a random glitch in the python 2.6 tests |
| @@ -211,12 +242,15 @@ def __init__(self, ax, loc=None, bbox=None, **kwargs): | |||
| self.set_clip_on(False) | |||
| self._cachedRenderer = None | |||
| self._cellType = 'default' | |||
| self._cellCreation = {"default": Cell, "scicell": SciCell} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is an instance level attribute. A class level would look something like
classfoo(object):bar= {'a':1,'b':2}def__init__(self,lab):iflabnotinself.bar:raiseValueError("invalid key")self.label=self.bar[lab]
Thebar attribute is shared between all instances of the class so changing it results in it changing forall instances. This can be useful if you want to set up a registry of availableCell types.
…es a class level dictionary and modified code to use it.
@jenshnielsen Thanks! @tacaswell OK, thanks for the feedback. I've made the necessary changes to use the new class dictionary. |
| @@ -181,6 +212,7 @@ class Table(Artist): | |||
| FONTSIZE = 10 | |||
| AXESPAD = 0.02 # the border between the axes and table edge | |||
| AVAILABLECELLTYPES = {"default": 0, "scicell": 1} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You can put theCell classes as values in this dictionary.
@tacaswell to clear up some confusion, we are going with pull request#4259 ? |
Can you work that out with@dkua and@KaiSong2014 ? |
@tacaswell we are going with@dkua pr. Thanks! |
...e default draw function. SciCell draws a polygon (line) instead of the rectangle. Extended Table to use the new SciCell class.#4044