jfree / jfreechart

A 2D chart library for Java applications (JavaFX, Swing or server-side).
http://www.jfree.org/jfreechart/
GNU Lesser General Public License v2.1
1.22k stars 461 forks source link

NaN Values in PolarPlots #409

Open SamKry opened 4 months ago

SamKry commented 4 months ago

Description There is a bug in the PolarPlots where NaN values are plotted incorrectly.

Steps to Reproduce Specific steps to reproduce the behavior:

  1. Set a value of the data points to Double.NaN
  2. Use the DefaultPolarItemRenderer and plot the data
  3. Make sure strokes are visible
  4. See the NaN value pushed the line to the top left corner (See Screenshot)

Expected behavior The NaN point should be skiped (linke Matplotlib for Python does).

Actual behavior The NaN point is pushed to the top left corner because translateToJava2D(...) returns (0.0, 0.0) as the coordinate.

Screenshots

image

As an example, this is how NaN values are currently handled in XYPlots:

image

Java version jdk-17.0.6.10

Additional context

The current implementation uses a poly which automatically connects the points. Since we want to skip points, we have to use the normal graphics object and do the drawing like in the XYPlots.

NOTE

I will create a PR with a fix for that issue.

trashgod commented 3 months ago

I am wary of silently skipping values. Might this feature see wider acceptance as a mutable property?

SamKry commented 3 months ago

I've looked into the code of XYPlots and pretty much implemented the renderer for PolarPlots the same way (see PR). A mutable property would of course be possible so we can control the behavior and also be backwards compatible.

But the current handling of NaN values is obviously wrong. They don't belong to the top left corner of a plot.

So there is still the question where NaN values should be displayed. In the center (0|0) would make more sense than on top-left.

Let me know what you think.

I could update the PR accordingly.

Have a nice day ☀️

trashgod commented 3 months ago

Thank you for responding. In either XYPlot or PolarPlot, I see no a priori way to distinguish between erroneous data values and intentionally NaN values. In XYPlot, a gap remains as a visible indicator of possible error; in PolarPlot, skipped values may obscure possible error such as erroneous conversion. Although I have never encountered the need for the proposed feature, I would argue against making it the default.

SamKry commented 3 months ago

I am sorry, i didn't meant skipping them. There is also a gap like in the XYPlots.

Before:

image

Same data points with the updated PolarPlot Renderer:

image

The NaN value creates a gap in the line. Is this what you were talking about?

trashgod commented 3 months ago

Yes, I believe so. Your images make it clear: in the first, I can see the NaN and work out how to handle it; in the second, I don't see anything unusual. In effect, information has been hidden.

I can't argue that the proposed feature would never be useful. As others may rely on the existing behavior, I would caution against making it a new default.

For reference, a similar issue arose in StandardXYItemRenderer, discussed here +ca. v.1.0.2.

Of course, I defer to the owner.

SamKry commented 3 months ago

Thank you for pointing out this discussion. I have checked out the StandardXYItemRenderer and as far as I see NaN values are handled the same as i suggested. They are not shown in the chart and there is no connecting line.

XYPlot with StandardXYItemRenderer :

image

Demo Code: ```java public static void main(String args[]) throws Exception { JFrame frame = new JFrame(); JPanel panel = new JPanel(); panel.setLayout(new BorderLayout()); XYSeries s1 = new XYSeries("Series 1", true, false); s1.add(2, 3.4); s1.add(3, 5.4); s1.add(4, Double.NaN); s1.add(5, 1.1); s1.add(6, Double.NaN); s1.add(7, Double.NaN); s1.add(8, 6.4); s1.add(9, 6.5); DefaultTableXYDataset dataset = new DefaultTableXYDataset(); dataset.addSeries(s1); XYDatasetTableModel tablemodel = new XYDatasetTableModel(); tablemodel.setModel(dataset); JTable dataTable = new JTable(tablemodel); JScrollPane scroll = new JScrollPane(dataTable); scroll.setPreferredSize(new Dimension(600, 150)); JFreeChart chart = ChartFactory.createXYLineChart( "XY Series Demo", "X", "Y", dataset, PlotOrientation.VERTICAL, true, true, false); StandardXYItemRenderer renderer = new StandardXYItemRenderer(); renderer.setDrawSeriesLineAsPath(true); // enable shapes renderer.setBaseShapesVisible(true); ((XYPlot) chart.getPlot()).setRenderer(renderer); ChartPanel chartPanel = new ChartPanel(chart); panel.add(chartPanel, BorderLayout.CENTER); panel.add(scroll, BorderLayout.SOUTH); frame.setContentPane(panel); frame.setSize(600, 500); frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); frame.show(); } ```

I understand that there might be users who got used to the way it was before where NaN values were plot in the top left corner.

Since i got used to the way NaN values are handled in XYPlots, I think it would make sense to have the same handling in all plots for consistency.

I could implement the new updated PolarPlot renderer so that only when calling a function like renderer.hideNaNValues(true) is called the new handling is used. So we still have the old handling with the NaN being at topleft as default but we can also switch to the new handling.

What do you think?

trashgod commented 3 months ago

Yes, a mutable propertye.g. your hideNaNValues set to false by default—would be less disruptive.

To be honest, I would be unlikely to use the feature. I typically handle anomalous values upstream of the data model, and then only having verified the desired functionality. For example, rather than compromise data integrity, I might propose an adjacent visual control to show or hide the NaN values.