Fireant 1.0 Rewrite #151
Merged
Fireant 1.0 Rewrite #151
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
twheys
force-pushed
the
fireant1.0
branch
4 times, most recently
from
1574328
to
f54bcee
Compare
twheys
force-pushed
the
fireant1.0
branch
5 times, most recently
from
9f10a67
to
f5937cd
Compare
twheys
force-pushed
the
fireant1.0
branch
6 times, most recently
from
91e4835
to
d13ed5d
Compare
x8lucas8x
reviewed
Jan 25, 2018
fireant/slicer/widgets/helpers.py
Outdated
| :param data_frame: | ||
| :return: | ||
| """ | ||
| dv_by_dimension = {} |
There was a problem hiding this comment.
No idea what dv stands for.
x8lucas8x
reviewed
Jan 25, 2018
fireant/slicer/widgets/highcharts.py
Outdated
|
|
||
| def _render_y_axis(self, axis_idx, color, references): | ||
| """ | ||
| Renders the yAxis configuraiton. |
There was a problem hiding this comment.
There is a typo in configuraiton
There was a problem hiding this comment.
good catch, will fix
x8lucas8x
reviewed
Jan 25, 2018
fireant/slicer/widgets/highcharts.py
Outdated
| """ | ||
| Renders the xAxis configuraiton. | ||
|
|
||
| https://api.highcharts.com/highcharts/yAxis |
There was a problem hiding this comment.
Good that you're putting the documentation link for the highcharts elements. That helps a lot when debugging or changing behaviour.
x8lucas8x
reviewed
Jan 25, 2018
fireant/slicer/widgets/highcharts.py
Outdated
|
|
||
| def _render_x_axis(self, data_frame, dimension_display_values): | ||
| """ | ||
| Renders the xAxis configuraiton. |
There was a problem hiding this comment.
There is a typo in configuraiton
x8lucas8x
reviewed
Jan 25, 2018
fireant/slicer/widgets/highcharts.py
Outdated
| "title": {"text": reference.label}, | ||
| "opposite": True, | ||
| "labels": {"style": {"color": color}} | ||
| } |
There was a problem hiding this comment.
Those 3 lines seem to be badly indented.
x8lucas8x
reviewed
Jan 25, 2018
fireant/slicer/widgets/highcharts.py
Outdated
|
|
||
| def _render_plot_options(self, data_frame): | ||
| """ | ||
| Renders the plotOptions configuration |
There was a problem hiding this comment.
Missing a dot in the end of the sentence. Don't think it's a big deal, but noticed you've put them for all other methods.
There was a problem hiding this comment.
Will fix. Looks better if it's consistent
…the pandas transformer
…frame that drops the metrics column index level when only a single metric is used. The changed accessor value was incorrect, the dropped metric name index level value is not reflected in the data.
Pandas sort columns
Fixed the replacement of display values on multiindex data frames in …
…y moving the fillna part after the sort
…gh charts transformer would raise an unhandled exception
Added handle for empty sort array in pandas transformer
Fix ReactTable sorting with NaNs
…es_for_highcharts Fixed a case where if a null was returned for a display value, the hi…
Handle invalid column idx in sort
…ame would mix data types, breaking the sort
…sformer when there are no dimensions
Fix fillna in pandas sortdataframe
…orting is not required
Fixed sorting data frames with no dimensions
…oltip to display the percentage total
Highcharts pie chart
…p matplotlib tests if matplotlib is not installed
Matplotlib widget
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rewrite of the entire package
Notable changes:
Still needs doing
Write highcharts transformerAdd tests for highcharts pie chartsWrite pandas transformerWrite csv transformerWrite matplotlib transformerAdd totalsAdd support for basic operations (cumsum, cumavg)Add fix for leap years to YoY reference queriesAdd paginationAdd function for querying dimension optionsAdapt documentation