Implemented issue 4044. Created a Cell subclass, SciCell, to override th...#4245
Implemented issue 4044. Created a Cell subclass, SciCell, to override th...#4245sushiblah wants to merge 6 commits intomatplotlib:masterfrom
Conversation
… the default draw function. SciCell draws a polygon (line) instead of the rectangle
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
This is an instance level attribute. A class level would look something like
class foo(object):
bar = {'a': 1, 'b': 2}
def __init__(self, lab):
if lab not in self.bar:
raise ValueError("invalid key")
self.label = self.bar[lab]The bar attribute is shared between all instances of the class so changing it results in it changing for all instances. This can be useful if you want to set up a registry of available Cell 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. |
There was a problem hiding this comment.
You can put the Cell 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 ? |
...e default draw function. SciCell draws a polygon (line) instead of the rectangle. Extended Table to use the new SciCell class. #4044