Class hierarchy fix and naming consistency #11
Conversation
We now subclass HighChartsView rathern than JSONView and call the appropraite methods on the superclass for things like context data & plot options.
jimr
changed the title
Mostly by making the base `get_context_data` do more work.
- Added a new chart type (area) - PEP8 function naming (get_xAxis -> get_x_axis) - Tidying in the base get_series method - Base get_context_data now does more work - added a 'chart_type' attribute
You can set the 'stacking' attribute on your chart class to be, e.g. 'percent' and the plot options will reflect this automatically. To do this we needed to switch to using defaultdicts rather than plain dicts and being a bit more careful in calling .update in subclasses.
Also return a defaultdict as the default y axis rather than None.
Moving towards a general approach of returning None from helpers to prevent them from being inserted into the context data where the base class can't meaningfully provide a default value (e.g. get_subtitle, get_drilldown, etc). For helpers where subclasses might call super (e.g. get_x_axis, we still return defualtdict objects.
Conflicts: chartjs/views/__init__.py
|
Nobody would agree more with me about how much this would be necessary. However, we would like a more strategic way of doing so than breaking backward compatibility. Could we have the renames and keep aliases/compatibility with the existing names ? |
|
I admit, the scope of this work changed rather dramatically as we increased our usage. If it was up to me, I'd accept the changes and bump the major version of the library to indicate the API breakage... If that's not possible, it ought to be straightforward enough to add helpers to preserve the old behaviour. |
|
Can you fix the tests? |
|
Are you interested in merging? |
|
It would be good to see this merged. |
|
@marauder37 would you like to take it over? Merge master, clean the tests and add backward incompatibility handling with DeprecationWarnings? |
We now have BaseColumnsHighChartsView subclass HighChartsView rathern than JSONView and call the appropraite methods on the superclass for things like context data & plot options.
Also updated a bunch of class names as per issue #12 and generally tried to improve consistency everywhere.