Skip to content
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

[WIP] numeric report charts #3528

Closed
wants to merge 21 commits into from

Conversation

martinpovolny
Copy link
Member

Work towards numeric report charts...

https://bugzilla.redhat.com/show_bug.cgi?id=1213294

chart_num_dim1
chart_num_dim2
chart_num_dim2-cols

@martinpovolny
Copy link
Member Author

chart-editor

@miq-bot
Copy link
Member

miq-bot commented Jul 20, 2015

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@martinpovolny
Copy link
Member Author

Flash version:

chart_num-flash1

@martinpovolny martinpovolny force-pushed the report_charts2 branch 3 times, most recently from 7f9d747 to d2c59cf Compare July 23, 2015 17:28
@martinpovolny martinpovolny changed the title [WIP] numeric report charts numeric report charts Jul 24, 2015
@martinpovolny martinpovolny force-pushed the report_charts2 branch 3 times, most recently from 8f9e290 to 5f9fc3b Compare July 24, 2015 11:12
@dclarizio
Copy link

Status update . . . trying to get some new field value based charts working, but will need to discuss with @martinpovolny to move forward. Marking as WIP until then.

@dclarizio dclarizio added the wip label Jul 29, 2015
@dclarizio dclarizio changed the title numeric report charts [WIP] numeric report charts Jul 29, 2015
@martinpovolny martinpovolny force-pushed the report_charts2 branch 2 times, most recently from 9e48c4f to c296561 Compare August 4, 2015 21:02
@dclarizio
Copy link

@martinpovolny Tested my simple Number of CPUs per provider report chart and found the following:

As noted prior, changing the Chart mode pull down to Counts, should not show the Data column pull down
chart mode drop down

Data column pull down should only show the columns available as Summary Calculation Rows
data_column_dropdown
In the shot below, I'm thinking we should show "Number of CPUs - Average" and "Number of CPUs - Total" as choices for the Data Column. Whichever is chosen is what is used for the chart.
summary calculations

Running the report results in the following chart showing
numcpus-chart1
This may be due to another bug where the actual report summary lines aren't being shown (also the same in master, guess I need to file a bug). There should be a Total summary line for each provider with the count value. If we had Average and Total checked, we should see 2 lines per provider. There is also an "All Rows" line at the bottom of the report, but it has no values showing either.
numcpus total line

@dclarizio
Copy link

@martinpovolny opened related issue #3738 which MAY be causing the charts to not have the proper data.

@martinpovolny
Copy link
Member Author

@dclarizio : I hacked in the simple usecase to make sure I understand what you want for that one.

on "As noted prior, changing the Chart mode pull down to Counts, should not show the Data column pull down" -- sure, will do that just want to make sure I have the general idea prior to playing with these small things...

on "Data column pull down should only show the columns available as Summary Calculation Rows"

ok, the fields will be limited to the ones that are used for sorting in the Summary page.

But: on the aggregations: see app/helpers/report_helper.rb -- I see no sense in showing the aggregation function "Total" or "Average" in simplest usecase, because we have no groups and run no aggregations over no groups.

If you try the more complex usecase (see the examples in the specs/... in this PR) you would run through the else branch in app/helpers/report_helper.rb and you would have the selection of the aggregate functions that are calculated over the groups.

But in the simple usecase you would have just one number as total or average, and it makes no sense to me to chart just one value.

I'll modify the form to match your feedback regarding selection of fieds and hiding of the row column and get back to you.

@dclarizio
Copy link

@martinpovolny ping me to discuss the summary based use case when you get a chance.

@miq-bot
Copy link
Member

miq-bot commented Aug 7, 2015

<gemfile_checker />Gemfile changes detected in commits martinpovolny@3e3acdf .. martinpovolny@76ac795. /cc @JPrause @simaishi

@miq-bot
Copy link
Member

miq-bot commented Aug 7, 2015

Checked commits martinpovolny@3e3acdf .. martinpovolny@d211fa3 with rubocop 0.32.1 and haml-lint 0.13.0
7 files checked, 27 offenses detected

app/controllers/report_controller/reports/editor.rb

lib/report_formatter/chart_common.rb

lib/report_formatter/jqplot.rb

  • 🔹 - Line 85, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for axis_category_labels_ticks is too high. [25.2/15]
  • 🔹 - Line 90, Col 121 - Metrics/LineLength - Line is too long. [124/120]
  • 🔹 - Line 138, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_reporting_chart_other_numeric is too high. [27.17/15]
  • 🔹 - Line 161, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for build_reporting_chart_dim2_numeric is too high. [22.2/15]

spec/lib/report_formater/jqplot_formater_spec.rb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants