Speed up graph objs #678
Speed up graph objs #678
Conversation
|
@Kully, wanna put some 👀 on this? //cc @depettit, thanks for the clear issue documentation. This targets the |
|
|
FWIW I always use the The added utility does have to be weighed up against another dependency but these days with conda I don't see that as such a big issue and it's already a dep for sever other widely used packages. |
|
Oops, thanks @dhirschfeld , I meant to preserve the docstrings but forgot 😓. I typically use something like wraps. What benefits beyond something like |
|
FWIW, these decorated functions aren't really meant to be user-facing, but, it was my intention to make a more general-purpose decorator that would preserve names/docs. |
|
OK, just read through the docs for decorate. Looks great! I'll try to
rewrite using this, seems like the dep is pretty small.
…On Jan 30, 2017 6:37 PM, "Dave Hirschfeld" ***@***.***> wrote:
FWIW I always use the decorator
<http://pythonhosted.org/decorator/documentation.html> module to
implement signature/docstring preserving decorators. If the decorated
functions aren't user-facing then it's possibly not a big issue however
I've found it useful when debugging my own code to have the decorated
functions have meaningful signatures and docstrings.
The added utility does have to be weighed up against another dependency
but these days with conda I don't see that as such a big issue and it's
already a dep for sever other widely used packages.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#678 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGTiWlyU3n8F61fN11Gba5JryQT9sFlMks5rXnRMgaJpZM4LyAWH>
.
|
|
The decorator module will preserve docstring and signature on both py2/3 and will ensure From a quick test, much of this is mitigated on py3.5 where wraps also preserves the signature and where My mantra of always using the |
The bottleneck for the `make_subplots` function was due to excessive lookups in the `plot_schema`. These lookups are actually pretty computation-intensive so caching these computations can give us a large performance boost. Note that Python 3.2+ has a new [`functools.lru_cache`](https://docs.python.org/3/library/functools.html#functools.lru_cache) which can be used for this. HOWEVER, we support Python 2.7+ and I didn’t see a backport for it. There are numerous `memoize` packages on PyPI, so many that I didn’t want to commit to one. It’s fairly simple to write this and then we don’t need another dependency.
The `value` argument currently allows for any value type. This isn’t very helpful for memoization. Instead, the top-level function now categorizes `value` as a string so that we *can* memoize the function. Also, we need hashable args, so we `tuple` the `parent_object_names`.
The `__init__` method ends up doing some additional validation, it’s more performant (and exactly the same), to set an item in layout as a dict instead of as a *graph_obj*.
theengineear
force-pushed
the
speed_up_graph_objs
branch
from
9ee5042 to
28c6fe0
Compare
|
@dhirschfeld , I took you up on the suggestion. Thanks! @chriddyp the |
|
Additionally, I should note that instead of letting callers specify a |
|
@Kully unless there's pushback on the new |
| @@ -1000,8 +1000,7 @@ def _get_anchors(r, c, x_cnt, y_cnt, shared_xaxes, shared_yaxes): | |||
| # Function pasting x/y domains in layout object (2d case) | |||
| def _add_domain(layout, x_or_y, label, domain, anchor, position): | |||
| name = label[0] + 'axis' + label[1:] | |||
| graph_obj = '{X_or_Y}Axis'.format(X_or_Y=x_or_y.upper()) | |||
| axis = getattr(graph_objs, graph_obj)(domain=domain) | |||
| axis = {'domain': domain} | |||
There was a problem hiding this comment.
I suppose you did this because axis will never be equal to anything other than {'domain': domain}, and so it's like a trivial case of memoization? :)
There was a problem hiding this comment.
this isn't about memoization as much as it's about performance. the line graph_obj = '{X_or_Y}Axis'.format(X_or_Y=x_or_y.upper()) was eating up a ton of cpu time because it was instantiating that graph_object and then immediately setting it inside layout, which effectively was instantiating it again.
|
|
||
| :param (int|None) maxsize: Limit the number of cached results. This is a | ||
| simple way to prevent memory leaks. Setting this | ||
| to `None` will remember *all* calls. The 128 |
There was a problem hiding this comment.
Do you think it's a good idea to allow None - i.e. remembering all calls? Perhaps to avoid huge memory leaks, maybe have the cap at a really high number.
There was a problem hiding this comment.
Yah, this mimics lru_cache. I'd have tried to use this, but it's not available in Python < 3.2.
| if key in keys: | ||
| return cache[key] | ||
|
|
||
| if maxsize is not None and len(keys) == maxsize: |
There was a problem hiding this comment.
This condition is set out very clearly! 👍
|
|
||
| self.assertEqual(name_space.call_count, 0) | ||
| for i, (inputs, result) in enumerate(tests, 1): | ||
| for _ in range(10): |
There was a problem hiding this comment.
Out of curiosity, is _ the canonical (or a typical) variable used for loops where the looping variable is not used inside?
There was a problem hiding this comment.
yup, _ is the common placeholder when you don't intent to use the value.
yup! |
Yup! 💃 🕺 👯 |
Introduces a utility decorator to memoize functions along with some other performance improvements surrounding `graph_reference.
This was something I tinkered on over the weekend. I spent a bit more time today formalizing it so that the work didn't go to waste.
Closes #497
^^ I'm closing that issue, but it's likely that there are other improvements to be had. My suggestion would be to separate
make_subplotsso that users can choose whether they'd like to usegraph_objsor native Python objects to create a plot. That can/should be handled as a different issue though.