Skip to content

Conversation

theengineear
Copy link
Contributor

Alright, the real deal!

TODO:

This has been changed in plotlyjs.
`Line` `opacity` has been removed (or never existed?). We keep the
functionality by merging the hex color and opacity into an rgba tuple.
This is used to take lowercased object names from our plot schema and
convert them to python-y class names.
`OBJECTS` maps the lower-cased object names in graph reference to an
iterable of paths in graph reference. For example, `marker` is found in
*many* places in the graph reference. Therefore, we have many paths that
can possibly define the contents of `marker`.
This is used to map a name like `Line` to the object `line` that exists
in graph reference.

For completeness, we then use `line` to lookup all the paths of `line`
in `OBJECTS` which tells us *all* the possible parents `line` can have
and all the attributes `line` can have given its parent.
The object is something like `marker` which *should* be looked up by
a path, E.g. `(‘traces’, ‘scatter’, ‘marker’)`. However, there are
caveats so we need to allow both a `path` and an `object_name` param.

`figure`, `data`, `layout`, and all the traces are special in that they
are either not strictly defined in the plot schema or they require
special functionality to gather their attributes.

`annotation` and `shape` are special in that their parent objects,
`annotations` and `shapes` both point to the same location in graph
reference. This means that we need *both* the name `annotation` and the
path to get our info.
For example, we need to define classes differently if the object is 
dict-like or list-like.
We used to change things like ‘scl’ to ‘colorscale’, but that info will
soon be added to the plot schema.
This used to be a method on the PlotlyObj, but it was static, which
means we can just move it. The old method isn’t being removed yet
because it’s too messy…
There’s no inherent order in the new plot schema so we need to sort
by *something*, if only for testing…

See #290.
`value_is_data` used the old `INFO` dict and the value of an attribute
to decide whether something had the `data` role or not. However, we can
just replace this with a general `get_role` function that can give the
default role or the ‘implicit’ role based on the value.
Setup for the refact of old PlotlyList/PlotlyDict classes.

This short-circuits some deprecated methods and provides a couple
general methods that all PlotlyList and PlotlyDict objects can use.
This will eventually replace the `get_class_instance_by_name` which was
a factory by the wrong name…
(these are short-circuited in the `PlotlyBase` class)
It’s in `graph_objs_tools.py` now.
`_name` refers to the object name (‘scatter’)
`_items` is a set of items that can go in the object (‘annotation’
    in ‘annotations’)
`_attributres` is a set of strings that can be used as attributes for
    the object (‘width’ for ‘line’)
`_parent_key` is the key that this object is defined under. Usually a
    bit redundant, but `textfont` for `Font` is a good example.
These basically replace the `to_graph_objs` method. Note that this only
needs to handle a single object because all objects will be PlotlyBase
objects now.
For example `Layout` can have `xaxis6` as an attr.
We override the dunder (double-underscore methods) to auto-convert
sub-dicts to graph objects. The important thing to look at is the
new `__setitem__` method.

Also, note that `__init__` now calls `append` which will funnel into
`__setitem__`.
Auto-complete sub-attrs. Keep things converted to graph objs *always*.

`__setitem__` is again the most important thing to look at!

Note that `__missing__` isn’t actually inherited, but exists in
`defaultdict`. Instead of inheriting from *that*, we just copy the idea.
Basically, this gets called when an attr or key can’t be found on the
object. It’s what does the auto-creation of things that don’t exist yet.

Also note the `__dir__` function which allows dynamic listing of
*possible* attributes on PlotlyDict objects. This is especially nice in
IPython Notebooks where you can just hit `TAB`.
@etpinard
Copy link
Contributor

@theengineear

@etpinard are these breaks correct? were any of these things deprecated?

These might be deprecated, but they are definitely not handled in cleanData at the moment.

I'd vote for updating cufflinks.

@theengineear
Copy link
Contributor Author

👍 sg!

@cldougl
Copy link
Member

cldougl commented Sep 21, 2015

It's working well for me- I think the help formatting looks good/clear!

@chriddyp
Copy link
Member

fixed the histogram line issue for cufflinks here: chriddyp/cufflinks@e66bafe, just waiting on a graph reference update.

It’s a maintenance issue to rely on `hrName` which `plotlyjs` doesn’t
really care about. We want to only depend on values that `plotlyjs` will
notice if changed.
This function should be deprecated anyhow…
This is in parallel for now to make the commits simpler.
Different from `string_to_class_name`, which is more generic. Since we
don’t auto-generate classes for all objects, we need to be smart about
which object_names get shown to users as `dict` and `list` vs a subclass
of `PlotlyList` and `PlotlyDict`.
We can now instantiate them with a `_name` kwarg, which is how we’ll
get around needing special classes for each object.
(Except for `Figure` and `Data`, which are patched)
@theengineear
Copy link
Contributor Author

@etpinard OK, trying to put your suggestions into motion. Here's how things work now:

image

Notes:

  • Adding/overwriting as a dict works now (just as it did before)
  • Adding/editing via the . works now (just as it did before)
  • The only new classes created are traces, no other sub-objects
  • Old sub-objects are still accessible for backwards compat
  • We use list or dict to explain sub-object instantiation to users. This is to hide the PlotlyDict and PlotlyList objects from users, who shouldn't know about these. Note that upon being added as a sub-object, a dict gets converted to a PlotlyDict with the appropriate _name and a list gets converted to a PlotlyList with an appropriate _name.
  • NO fancy classes are actually used in sub-objects anymore. They're all generic PlotlyList or PlotlyDict objects with some _name and an awareness of their place in a figure.
  • Exception to the above for Figure and Data, which we patch, meaning we need to use the actual sub-classes defined in the graph_objs module.

Questions:

  • Internally, should we now change to never using things like Marker, Data, etc? We can just instantiate from Figure, use dict/list or use GraphObjectFactory.create with a hidden _name argument, which seems alright to do internally. E.g. GraphObjectFactory.create(_name='marker'). I wouldn't mind if all our object creation went through this... it also makes our reference errors go away for dev stuff.
  • Should we not expose objects like Marker in the to_string call? I think we should! It has the unexpected, but nice side effect of making the instantiation path errors auto-magically work out :) **see below

**below

When we have special classes instantiated as sub-objects we can't easily get the path to the error...

image
image

However, when we start the instantiation from the top by using dict objects:

image
image

We get the full error path, nice!

It's sorta ugly though, imo:

image

@theengineear
Copy link
Contributor Author

Thoughts on trying to get this merged and done with?

@etpinard
Copy link
Contributor

Internally, should we now change to never using things like Marker, Data, etc? We can just instantiate from Figure, use dict/list or use GraphObjectFactory.create with a hidden _name argument, which seems alright to do internally. E.g. GraphObjectFactory.create(_name='marker'). I wouldn't mind if all our object creation went through this... it also makes our reference errors go away for dev stuff.

Yes. In version 2.0.0 I'd vote for having Marker, Data and all the graph objects other than the trace objects, Layout and Figure out of the picture. Using GraphObjectFactory.create sounds like the easiest and most scalable way for keeping the python api in-sync.

Should we not expose objects like Marker in the to_string call? I think we should! It has the unexpected, but nice side effect of making the instantiation path errors auto-magically work out :) **see below

I'd vote for the full-path solution. Plotly attributes are setup to depend on the root object; python api users should get to know that fact.

In that case, how hard would it be to change the error message

image

so that things like PlotlyDict aren't exposed to the users?

# Moving forward, we only add new trace names.
# {<ClassName>: {'object_name': <object_name>, 'base_type': <base-type>}
_BACKWARDS_COMPAT_CLASS_NAMES = {
'AngularAxis': {'object_name': 'angularaxis', 'base_type': dict},
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. Maybe @chriddyp could use this for the https://plot.ly/python/reference/

@etpinard
Copy link
Contributor

💃 The dream is now reality.

@chriddyp
Copy link
Member

💃 looks good to me!

@theengineear
Copy link
Contributor Author

💥 thanks guys. i'll clean up some things related to #292 (comment) tomorrow and merge this guy in! woot!

@theengineear
Copy link
Contributor Author

second thought, this pr is wayy to massive to keep working on. i'll open up new changes in new prs.

theengineear added a commit that referenced this pull request Sep 26, 2015
@theengineear theengineear merged commit a3b4023 into master Sep 26, 2015
@theengineear theengineear deleted the update-graph-reference branch September 26, 2015 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants