Fix #47: Support NaN values for disconnected line segments

[krystophny]

User description

Summary

• Implements NaN support in add_plot to allow disconnected line segments
• Resolves issue Multiple add_plot calls connect disconnected line segments #47 where multiple add_plot calls would connect separate line segments
• Users can now insert NaN values in their data arrays to create line breaks

Implementation Details

The solution modifies the line rendering routines to check for NaN values:

• render_solid_line: Skips any segment where either endpoint contains NaN
• render_patterned_line: Handles NaN by resetting pattern state and skipping segments
• render_markers: Skips data points with NaN values

All backends (PNG, PDF, ASCII) automatically inherit this behavior since they use the same rendering logic.

Test Plan

• Added test_disconnected_lines.f90 with two test cases:
  • Multiple add_plot calls showing the original issue
  • Single add_plot with NaN separators showing the fix
• Added example disconnected_lines.f90 demonstrating practical usage
• Verified all backends produce correct output with disconnected segments
• Existing tests continue to pass

Example Usage

use, intrinsic :: ieee_arithmetic, only: ieee_value, ieee_quiet_nan
real(8) :: x(5), y(5), nan

nan = ieee_value(nan, ieee_quiet_nan)

\! Two segments separated by NaN: (0,0)-(1,0) and (2,1)-(3,1)
x = [0.0_8, 1.0_8, nan, 2.0_8, 3.0_8]
y = [0.0_8, 0.0_8, nan, 1.0_8, 1.0_8]

call fig%add_plot(x, y)

Closes #47

🤖 Generated with Claude Code

---

PR Type

Bug fix, Enhancement

---

Description

• Implement NaN support for disconnected line segments

• Skip NaN values in line rendering and markers

• Add comprehensive tests and examples

• Support all backends (PNG, PDF, ASCII)

---

Diagram Walkthrough

flowchart LR
  A["Data with NaN"] --> B["render_solid_line"]
  A --> C["render_patterned_line"]
  A --> D["render_markers"]
  B --> E["Skip NaN segments"]
  C --> F["Reset pattern state on NaN"]
  D --> G["Skip NaN points"]
  E --> H["Disconnected lines"]
  F --> H
  G --> H

Loading

File Walkthrough

	Relevant files
Bug fix	fortplot_figure_core.f90 Implement NaN handling in line rendering                                  src/fortplot_figure_core.f90 Add NaN checks in render_solid_line to skip invalid segments Modify render_patterned_line to handle NaN with pattern state reset Update render_markers to skip NaN data points Import ieee_arithmetic module for NaN detection +37/-6
Tests	test_disconnected_lines.f90 Add disconnected lines test cases                                                test/test_disconnected_lines.f90 Add test for multiple plots that shouldn't connect Add test for NaN values breaking line segments Generate PNG outputs for visual verification Use ieee_arithmetic for NaN value creation +56/-0
Documentation	disconnected_lines.f90 Add disconnected lines usage example                                          example/disconnected_lines.f90 Create example with three disconnected segments using NaN separators Demonstrate sine, cosine, and horizontal line segments Show single point plotting capability Generate outputs in PNG, PDF, and ASCII formats +63/-0

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 47 - PR Code Verified Compliant requirements: • Fix issue where multiple add_plot calls connect disconnected line segments • Support NaN values as line breaks (like matplotlib) • Ensure two separate horizontal line segments remain disconnected • Provide solution for plotting mesh triangles without spurious connections Requires further human verification: • Visual verification that PNG/PDF outputs show properly disconnected segments • Testing with actual mesh triangle plotting use case from FortFEM

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Performance Impact The NaN checking logic is added to every segment rendering loop which could impact performance for large datasets. Consider if the NaN checks could be optimized or cached. if (ieee_is_nan(self%plots(plot_idx)%x(i)) .or. ieee_is_nan(self%plots(plot_idx)%y(i)) .or. & ieee_is_nan(self%plots(plot_idx)%x(i+1)) .or. ieee_is_nan(self%plots(plot_idx)%y(i+1))) then cycle end if Memory Allocation The render_patterned_line function allocates additional arrays for valid_points tracking but the cleanup only deallocates some arrays. Verify all allocated memory is properly freed. logical, allocatable :: valid_points(:) ! Transform all data points to get proper scaling allocate(x_trans(size(self%plots(plot_idx)%x))) allocate(y_trans(size(self%plots(plot_idx)%y))) allocate(valid_points(size(self%plots(plot_idx)%x))) valid_count = 0 do i = 1, size(self%plots(plot_idx)%x) valid_points(i) = .not. (ieee_is_nan(self%plots(plot_idx)%x(i)) .or. ieee_is_nan(self%plots(plot_idx)%y(i))) if (valid_points(i)) then x_trans(i) = apply_scale_transform(self%plots(plot_idx)%x(i), self%xscale, self%symlog_threshold) y_trans(i) = apply_scale_transform(self%plots(plot_idx)%y(i), self%yscale, self%symlog_threshold) valid_count = valid_count + 1 else x_trans(i) = 0.0_wp y_trans(i) = 0.0_wp end if end do x_range = maxval(x_trans) - minval(x_trans) y_range = maxval(y_trans) - minval(y_trans) plot_scale = max(x_range, y_range) ! Define pattern lengths (matplotlib-like) dash_len = plot_scale * 0.03_wp ! 3% of range dot_len = plot_scale * 0.005_wp ! 0.5% of range gap_len = plot_scale * 0.015_wp ! 1.5% of range ! Define patterns like matplotlib select case (trim(linestyle)) case ('--') ! Dashed: [dash, gap, dash, gap, ...] pattern_size = 2 pattern(1) = dash_len ! dash pattern(2) = gap_len ! gap case (':') ! Dotted: [dot, gap, dot, gap, ...] pattern_size = 2 pattern(1) = dot_len ! dot pattern(2) = gap_len ! gap case ('-.') ! Dash-dot: [dash, gap, dot, gap, dash, gap, dot, gap, ...] pattern_size = 4 pattern(1) = dash_len ! dash pattern(2) = gap_len ! gap pattern(3) = dot_len ! dot pattern(4) = gap_len ! gap case default ! Unknown pattern, fall back to solid call render_solid_line(self, plot_idx) deallocate(x_trans, y_trans) return end select ! Calculate total pattern length pattern_length = sum(pattern(1:pattern_size)) ! Render with continuous pattern current_distance = 0.0_wp pattern_index = 1 drawing = .true. ! Start drawing do i = 1, size(self%plots(plot_idx)%x) - 1 ! Skip segment if either point is invalid (NaN) if (.not. valid_points(i) .or. .not. valid_points(i+1)) then ! Reset pattern state when encountering NaN current_distance = 0.0_wp pattern_index = 1 drawing = .true. cycle end if x1_screen = x_trans(i) y1_screen = y_trans(i) x2_screen = x_trans(i+1) y2_screen = y_trans(i+1) dx = x2_screen - x1_screen dy = y2_screen - y1_screen segment_length = sqrt(dx*dx + dy*dy) if (segment_length < 1e-10_wp) cycle call render_segment_with_pattern(self, x1_screen, y1_screen, x2_screen, y2_screen, segment_length, & pattern, pattern_size, pattern_length, & current_distance, pattern_index, drawing) end do ! Clean up deallocate(x_trans, y_trans, valid_points)

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Handle all-NaN data gracefully Suggestion Impact: The suggestion was almost directly implemented. The commit adds the same conditional check for valid_count > 0, uses masks for minval/maxval calculations, and provides a default scale value. The only difference is that the commit sets plot_scale directly to 1.0_wp instead of setting x_range and y_range to 1.0_wp separately. code diff: + ! Handle case where all points are NaN + if (valid_count > 0) then + x_range = maxval(x_trans, mask=valid_points) - minval(x_trans, mask=valid_points) + y_range = maxval(y_trans, mask=valid_points) - minval(y_trans, mask=valid_points) + plot_scale = max(x_range, y_range) + else + ! All points are NaN, use default scale + plot_scale = 1.0_wp + end if When all points are NaN, x_trans and y_trans arrays contain only zeros, making maxval and minval both zero, resulting in zero ranges. This could cause division by zero or scaling issues later in the code. src/fortplot_figure_core.f90 [1480-1481] -x_range = maxval(x_trans) - minval(x_trans) -y_range = maxval(y_trans) - minval(y_trans) +if (valid_count > 0) then + x_range = maxval(x_trans, mask=valid_points) - minval(x_trans, mask=valid_points) + y_range = maxval(y_trans, mask=valid_points) - minval(y_trans, mask=valid_points) +else + x_range = 1.0_wp + y_range = 1.0_wp +end if [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that minval and maxval will be calculated on an array containing zeros for NaN points, which can lead to an incorrect plot scale, and provides a robust fix using a mask.	Medium
	Fix incorrect array indexing The cosine calculation uses incorrect indexing. The expression x(i-5+1) simplifies to x(i-4), which doesn't correspond to the intended x-values for the cosine wave. Use the actual x-values from the array instead. src/fortplot_figure_core.f90 [29-31] do i = 6, 9 - y(i) = cos(x(i-5+1)) + y(i) = cos(x(i)) end do [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a bug in the example code where incorrect indexing x(i-5+1) would produce a distorted plot; however, it incorrectly identifies the file as src/fortplot_figure_core.f90.	Medium
Update

[codecov]

Codecov Report

Attention: Patch coverage is 79.41176% with 35 lines in your changes missing coverage. Please review.

Project coverage is 64.26%. Comparing base (cd503bc) to head (6e468ce).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
example/disconnected_lines.f90	0.00%	31 Missing ⚠️
src/fortplot_figure_core.f90	84.00%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
+ Coverage   63.42%   64.26%   +0.84%     
==========================================
  Files         142      144       +2     
  Lines       11493    11658     +165     
  Branches     1971     1977       +6     
==========================================
+ Hits         7289     7492     +203     
+ Misses       3262     3218      -44     
- Partials      942      948       +6     

Flag	Coverage Δ
unittests	64.26% <79.41%> (+0.84%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.