Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update graph reference #292

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

Merged
theengineear merged 113 commits into master from update-graph-reference
Sep 26, 2015
Merged

Update graph reference #292

theengineear merged 113 commits into master from update-graph-reference
Sep 26, 2015

Conversation

@theengineear
Copy link
Contributor

@theengineear theengineear commented Sep 15, 2015

Alright, the real deal!

TODO:

`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`.
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.

Copy link
Contributor Author

👍 sg!

Copy link
Member

cldougl commented Sep 21, 2015

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

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)
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

Copy link
Contributor Author

Thoughts on trying to get this merged and done with?

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?

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/

Copy link
Contributor

💃 The dream is now reality.

Copy link
Member

💃 looks good to me!

Copy link
Contributor Author

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

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /