-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Traitlets #4762
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
Traitlets #4762
Conversation
in_range = False | ||
if value is None or value is False or value in ['none','']: | ||
# Return transparent if no other default alpha was set | ||
return (0.0, 0.0, 0.0, 1.0) |
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 black and opaque, not transparent. I think you want all zeros.
General strategy question: do you expect to be able to get to the test-and-fix stage (4) with one or a few traits at a time, or do you have to get everything in place before you can make anything work? |
|
||
stale = Bool(True) | ||
|
||
def _stale_changed(self): |
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 should only trigger on the False
-> True
transistion
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.
Good point. Missed that in the last branch too.
@efiring I'm hoping to be able to do things in batches. Right now I'm working with |
I would start with something simpler, like color or zorder rather than On Thu, Jul 23, 2015, 2:44 PM Ryan Morshead notifications@github.com
|
On 2015/07/23 8:44 AM, Ryan Morshead wrote:
Good. I advocate small batches for exactly that reason: much easier to Do you have any way of evaluating the performance consequences of these |
On 2015/07/23 8:46 AM, Thomas A Caswell wrote:
Good idea, though I think that color is actually one of the most |
One argument for doing "stale", "transform", etc first is that these are On Thu, Jul 23, 2015 at 2:55 PM, Eric Firing notifications@github.com
|
|
Thanks for the benchmark results. About a 14% penalty so far, in this On 8/2/15, Ryan Morshead notifications@github.com wrote:
|
@efiring test code was taken directly from |
67d84aa
to
e764f37
Compare
Rebased
Haven't investigated yet, but any chance that's coming from the changes to |
@rmorshea If that is happening locally, then the problem is out-dated c-cextensions (there was some recent work to improve the pdf backends handling of raster data). |
Is that something that requires work on your end to fix? Also, I'm presuming that Travis CI won't be able to build this branch until traitlets is officially supported? |
Try deleting the build directory and rebuilding. distutils doesn't do proper dependency resolution for C++ headers. |
from .transforms import IdentityTransform, Transform | ||
import contextlib | ||
|
||
class exdict(dict): |
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.
Might want to inherit from userdict
9784b8c
to
d09e70e
Compare
I got the same sort of I can reproduce the sphinx issue locally, but no real guess at what is causing it. I don't think it is something maformed with that intermediate output of the rst file, but I don't know where shpnix keeps them to check... |
@@ -575,9 +802,14 @@ def get_sketch_params(self): | |||
|
|||
May return `None` if no sketch parameters were set. | |||
""" | |||
return self._sketch | |||
msg = ("This has been deprecated to make way for IPython's" |
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.
Maybe remove the IPython's
here (and elsewhere similar messages appear). This is not to avoid giving credit where credit is due, just that this could imply that we are growing a dependency on IPython, which we aren't.
Frustratingly, I can't reproduce the I'm looking into the Sphinx issue now... |
I'm running Traitlets off |
I copy-pasted the pip source install from .travis.yml. |
I'm going to finish rebasing (as best I can), and see where I'm at: I feel like I've learned so much since I began this that it might be worth tearing everything down to start again with better practices and better ideas. If I had the time, I'd start from a fresh fork, but since I don't I'm hesitant. I'll try to figure that out this week though. |
@rmorshea my impression is that there have been some big changes in traitlets since you started. Is this correct, and if so were they early enough in the process that you were able to take advantage of them? In any case, I'm glad you are able to get back to this now. |
Answering my own question, it looks like the major change was the introduction of decorators, which you are already using. So the difficulties are because of all the mpl changes in the interim. |
@efiring, yes. The main outward change to Traitlets has been the new decorator API. Current plan: rebuild from the ground up, but repurpose my past work wherever possible. There are three reasons that I want to do this (in order of importance):
|
Rebase is unreasonably hairy - closing pending #8035 corresponding PR. |
This can be revisited if someone takes up the traitlets torch |
@rmorshea Can you elaborate on this one? |
@Thisch it's been a really long time since I've looked at this so without looking I'm not quite sure what parts of traitlets I might have wanted to leverage. If I had to guess it probably has to do with what's included in the traitlets.py file. I'm happy to advise anyone whose willing to take up the torch since I don't have the time to do it myself. |
Refactoring of Matplotlib with traitlets. See TODO for progress.
This change is