Auto/scatter basic 006

[MarkusNeusinger]

No description provided.

[Copilot]

Pull request overview

This PR adds a new visualization specification and implementations for "scatter-basic-006", which creates multiple scatter plots (x vs y1, y2, y3) in a single figure using both matplotlib and seaborn libraries. The specification defines how to compare multiple relationships using scatter plots with consistent styling and clear visual differentiation between series.

Key Changes:

• New specification file defining requirements for multi-series scatter plots
• Matplotlib implementation providing the core functionality
• Seaborn implementation offering an alternative with seaborn-specific styling options

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

File	Description
specs/scatter-basic-006.md	Specification document defining requirements for multiple scatter plots in single figure (missing template metadata and using non-standard section structure)
plots/seaborn/scatterplot/scatter-basic-006/default.py	Seaborn implementation with parameter validation, styling options, and three-series plotting (has bugs with deprecated API and missing validation)
plots/matplotlib/scatter/scatter-basic-006/default.py	Matplotlib implementation with parameter validation and three-series plotting (has bug with missing validation and minor maintainability issues)

Comments suppressed due to low confidence (2)

plots/matplotlib/scatter/scatter-basic-006/default.py:95

• Variable scatter1 is not used.

    scatter1 = ax.scatter(

plots/matplotlib/scatter/scatter-basic-006/default.py:104

• Variable scatter2 is not used.

    scatter2 = ax.scatter(

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

[Copilot]

Missing validation for the colors parameter. If a user provides a list with fewer than 3 colors, the code will raise an IndexError when accessing colors[1] or colors[2] (lines 119, 131). Add validation after line 85:

if colors is None:
    colors = ["blue", "orange", "green"]
elif len(colors) < 3:
    raise ValueError(f"colors must contain at least 3 colors, got {len(colors)}")

Suggested change

	colors = ["blue", "orange", "green"]
	colors = ["blue", "orange", "green"]
	elif len(colors) < 3:
	raise ValueError(f"colors must contain at least 3 colors, got {len(colors)}")

[Copilot]

The specification file is missing the template version metadata that is required by the spec template (version 1.0.0). Based on other spec files in the codebase (e.g., scatter-basic-001.md), the file should include:

<!--
Spec Template Version: 1.0.0
Created: YYYY-MM-DD
Last Updated: YYYY-MM-DD
-->

**Spec Version:** 1.0.0

after the title and before the Description section.

Suggested change

	<!--
	Spec Template Version: 1.0.0
	Created: 2024-06-10
	Last Updated: 2024-06-10
	-->
	**Spec Version:** 1.0.0

[Copilot]

sns.reset_orig() was deprecated in seaborn 0.8 and removed in seaborn 0.12+. This will cause an AttributeError with current versions of seaborn. Consider using a context manager instead:

with sns.axes_style(style):
    # Create figure and plot
    fig, ax = plt.subplots(figsize=figsize)
    # ... plotting code ...

Alternatively, use sns.reset_defaults() if you need to reset settings, though using a context manager is the preferred approach as it doesn't have side effects.

Suggested change

	sns.scatterplot(
	data=data,
	x=x,
	y=y3,
	s=point_size,
	alpha=alpha,
	color=colors[2],
	label=y3,
	ax=ax,
	**kwargs
	)
	# Apply styling
	ax.set_xlabel(xlabel, fontsize=12)
	ax.set_ylabel(ylabel, fontsize=12)
	ax.set_title(title, fontsize=14, fontweight='bold')
	# Customize grid if needed (seaborn already adds grid with whitegrid style)
	if grid:
	ax.grid(True, alpha=grid_alpha, linestyle='--')
	else:
	ax.grid(False)
	# Ensure legend is properly positioned
	ax.legend(loc=legend_loc, framealpha=0.9)
	# Ensure layout doesn't cut off labels
	plt.tight_layout()
	# Reset seaborn style to avoid affecting other plots
	sns.reset_orig()
	with sns.axes_style(style):
	sns.scatterplot(
	data=data,
	x=x,
	y=y3,
	s=point_size,
	alpha=alpha,
	color=colors[2],
	label=y3,
	ax=ax,
	**kwargs
	)
	# Apply styling
	ax.set_xlabel(xlabel, fontsize=12)
	ax.set_ylabel(ylabel, fontsize=12)
	ax.set_title(title, fontsize=14, fontweight='bold')
	# Customize grid if needed (seaborn already adds grid with whitegrid style)
	if grid:
	ax.grid(True, alpha=grid_alpha, linestyle='--')
	else:
	ax.grid(False)
	# Ensure legend is properly positioned
	ax.legend(loc=legend_loc, framealpha=0.9)
	# Ensure layout doesn't cut off labels
	plt.tight_layout()
	# No need to reset seaborn style; context manager handles it

[Copilot]

Setting the seaborn style with sns.set_style(style) at line 95 has global side effects that can affect other plots in the same process. Combined with the problematic sns.reset_orig() on line 155, this creates potential issues. Consider using a context manager to apply the style locally:

with sns.axes_style(style):
    fig, ax = plt.subplots(figsize=figsize)
    # ... rest of plotting code ...

This ensures the style only affects this specific plot without modifying global state.

[Copilot]

plt.tight_layout() is called on the global pyplot state rather than on the figure object. While this works, it's better practice to use fig.tight_layout() to avoid potential side effects on other figures. Change to:

fig.tight_layout()

Suggested change

	plt.tight_layout()
	fig.tight_layout()

[Copilot]

plt.tight_layout() is called on the global pyplot state rather than on the figure object. While this works, it's better practice to use fig.tight_layout() to avoid potential side effects on other figures. Change to:

fig.tight_layout()

Suggested change

	plt.tight_layout()
	fig.tight_layout()

[Copilot]

Missing validation for the colors parameter. If a user provides a list with fewer than 3 colors, the code will raise an IndexError when accessing colors[1] or colors[2] (lines 108, 117). Add validation after line 82:

if colors is None:
    colors = ["blue", "orange", "green"]
elif len(colors) < 3:
    raise ValueError(f"colors must contain at least 3 colors, got {len(colors)}")

Suggested change

	colors = ["blue", "orange", "green"]
	colors = ["blue", "orange", "green"]
	elif len(colors) < 3:
	raise ValueError(f"colors must contain at least 3 colors, got {len(colors)}")

[Copilot]

The specification file uses non-standard section names. According to the spec template (version 1.0.0), the standard sections should be:

• "Data Requirements" (not "Data Format")
• "Optional Parameters" (currently missing)
• "Quality Criteria" (not "Visual Elements")
• "Expected Output" (currently missing)
• "Tags" (currently missing)
• "Use Cases" (currently titled "Example Use Case")

Consider restructuring to align with the template for consistency across specifications.

[Copilot]

[nitpick] The variables scatter1, scatter2, and scatter3 are assigned but never used. Since the scatter plot objects are not needed after creation (labels are passed directly and the legend uses those labels), these assignments can be simplified by not capturing the return values:

ax.scatter(
    data[x], data[y1],
    s=point_size,
    alpha=alpha,
    color=colors[0],
    label=y1,
    **kwargs
)

[Copilot]

Variable scatter3 is not used.