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

Fix aggregate chart xaxis labels #1922

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aestoltm
Copy link
Contributor

@aestoltm aestoltm commented Oct 7, 2024

Description

Fix aggregate chart xaxis labels when multiple statistics are plotted for a single yaxis object. The issue was that with multiple aggregate data series, the first trace determines the x axis labels for the chart. If the following data series objects contain xvalues that are not within the first trace then those values are ignored (null'ed).

The following snippet shows how the xvalues for each yaxis object starts as an empty array of null values. Values are only updated if a match exists within the xAxis object. Looking at getYAxis() in ComplexDataset.php

$xValues = array_fill(
                    0,
                    $this->_xAxisDataObject->getCount(),
                    null
                );
foreach ($subXAxisObject->getValues() as $xIndex => $xValue) {
                    $found = array_search(
                        $xValue,
                        $this->_xAxisDataObject->getValues(),
                        true
                    );

                    if ($found !== false) {
                        $newValues[$found] = $yAxisDataObject->getValue($xIndex);
                        $xIds[$found]      = $subXAxisObject->getId($xIndex);
                        $xValues[$found]   = $subXAxisObject->getValue($xIndex);

                        if (
                            $dataDescripterAndDataset->data_description
                                                     ->std_err
                        ) {
                            $newErrors[$found]
                                = $yAxisDataObject->getError($xIndex);
                        }
                    }
                } // foreach ($subXAxisObject->getValues() ...

The reason this was not an issue before is that Highcharts set categorical axis labels through the xAxis object, whereas, Plotly JS has its category labels set through the data object itself. This should have been changed from the start of the transistion to Plotly to use the xAxis object to receive the xAxis values.

Before:
chart_before

After:
chart_after

Motivation and Context

Aggregate charts that contain multiple statistics at once will have blank labels if the label does not exist in every trace.

Tests performed

Tested on my dev port

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

…y from xAxis object instead of using yAxisObject->getXValue()
@aestoltm aestoltm added the bug Bugfixes label Oct 7, 2024
@aestoltm aestoltm added this to the 11.0.1 milestone Oct 7, 2024
@aestoltm aestoltm self-assigned this Oct 7, 2024
@jpwhite4
Copy link
Member

jpwhite4 commented Oct 7, 2024

There seem to be more bars in the second plot than the first one. How come?

@aestoltm
Copy link
Contributor Author

aestoltm commented Oct 7, 2024

There seem to be more bars in the second plot than the first one. How come?

It looks like there are more bars on xdmod-dev even without the proposed changes.

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

Successfully merging this pull request may close these issues.

2 participants