Make graph_objs.py compatible with new graph reference!

[etpinard]

• created from @theengineear 's factories branch
• adapt graph reference INFO to new graph reference structure
• add doc content (e.g. links to plot.ly examples and parent keys for a given obj)

[etpinard]

@theengineear this is not ready to merge yet

Requirements:

• Update the makefile (I think it should only sync the python-specific graph reference json files i.e. not the whole graph reference repo).
• Create doc (i.e. key info) for Layout, Figure
• Make sure this branch passes the tests
• ...

[etpinard]

@theengineear because of this statement, Layout, Data and Figure are not generated with type() and hence without make_dict_doc.

I think you did this because Layout, Data and Figure have a few special methods that use didn't want to overwrite.

I'm thinking instead of defining those three objects with type() first and then add the their special methods using something like this.

Thoughts?

[theengineear]

@etpinard : re-Layout, Data, Figure. Yes, I think that makes sense to do it as you mention. In fact, I think it's clearer to do so this way since it is a patch to keep things working. This is best done is a clear, separate place in the code.

[theengineear]

In general, i think we break enough backwards compatibility (e.g, maybe users were using INFO for something) here that we might want to make this a bump to 1.3.0. Additionally, it's a pretty big overhaul of how we piece together our modules. I'd like to (sorry!) string this out a bit more? Maybe we can make a proper 1.3.0 branch for us to start collaborating on? Then we can make PRs to this branch instead of being worried about both working in the same branch.

[theengineear]

Also, nice job!

[etpinard]

@theengineear good call on bumping to v. 1.3.0 !

How should we go about this?

I'm thinking that the easiest way would be to make the 1.3.0 branch off etienne-factories, merging master and got from there. Thoughts?

In the meantime,

• I'll attempt patch Data, Figure and Layout as you proposed
• I'll remove PlotlyDict, PlotlyList and other Python-specific graph objs from graph reference (I'll push to https://github.com/plotly/graph_reference/pull/17)

[etpinard]

@theengineear A quick question for you:

• Do you still see the PlotlyTrace primitive graph object as relevant ?

Looks like it only overrides the .to_string() method, removing 'type' in non-Trace inherited objects.

I just want to make sure that you still see PlotlyTrace as relevant before modifying OBJ_MAP accordingly. Thanks in advance.

[theengineear]

TL;DR it's easy to test if isinstance(obj, PlotlyTrace): ...

@etpinard , a lot of the logic in the python library depends on inheritance. I like this approach and i think it seems pythonic enough, but I'm open to discussion. We could alternatively just have a property set in all the trace objects to 'trace' or something. I think PlotlyTrace is a relevant member of the inheritance chain there, but I Trace is still certainly in question.

Here's an example of how it's used:
https://github.com/plotly/python-api/blob/master/plotly/graph_objs/graph_objs.py#L954-960

[etpinard]

Oh. Sorry I didn't see that ref to PlotlyTrace. I'll keep it in and modify OBJ_MAP no problem.

[etpinard]

@theengineear I merged master and I got all the tests to work!

Wanna merge this and bump to version 1.3.0 ?

[theengineear]

@etpinard re-merging to master. I'd like to get the calls to things like globals() and locals() out of these files before releasing. That's hella-confusing to debug. I was using them as an easy way to get at defined classes from strings. but we should be more explicit. Here are examples of the usage:

https://github.com/plotly/python-api/blob/c86fd287f15ae71551344bc4922f6ca4cc2b55c4/plotly/graph_objs/graph_objs.py#L290

https://github.com/plotly/python-api/blob/c86fd287f15ae71551344bc4922f6ca4cc2b55c4/plotly/graph_objs/graph_objs.py#L989-996

I think there are a couple of stability issues I'd like to clean up in 1.2.x and then push a final version to PyPI before this change (which i think it fairly big).

Is this holding back 3D right now?

[etpinard]

@theengineear Correct.

Ideally, I would love to generate the new 3D graph objects with using the machinery in this PR.

That said, I have to fill up graph_reference/ first, so it is not really "blocking" anything at the moment.

[etpinard]

@theengineear Also can I ask ... is there an easy way to remove substitute the globals() expressions in the cases you mentioned above?

[theengineear]

maybe this would be more easily read if:

def get_class_instance_by_name(name, *args, **kwargs):

That might be too long now, but _factory or factory are a maybe a little confusing.

[etpinard]

Good call.

[theengineear]

@etpinard , Re: tests, yeah, might just be a connection error, i should go back in and make things a bit more robust... i'd like to refactor the test suite to be a little smarter at some point, definitely not priority though. maybe just rebuild after all the changes are made? Are they passing when you run them on your machine?

[etpinard]

@theengineear yeah, running $ nosetests on my machine yields no errors.

[theengineear]

@etpinard , yeah i think this is looking great! there might be some merge conflicts at this point, but they shouldn't be a big deal.

👯 once these things are changed and tests pass on circle, feel free to ping me about anything.

[theengineear]

k, yeah, i've seen tests intermittently fail, i'm sure it's no big deal, as long as you can dev/test on your machine OK, we can just figure out circle before we merge this into master.

[chriddyp]

tldr what just happened?

[theengineear]

@chriddyp , python library got an updated graph reference so 3d works. plus, we refactored how classes are generated so that almost none of the classes are hardcoded. this is in an attempt to make adding things in graph reference and having them show up in the python library more seamless.

[etpinard]

3d graph objs, auto-generated by new framework.