Add majorDiameter and minorDiameter to ellipse fns

[benjamaan476]

To align the ellipse fns with circle, you can now specify the major/minor diameter of the ellipse instead of the radius

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
modeling-app	Ready	Preview	Comment	Sep 4, 2025 11:51am

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #8057 will not alter performance

_{Comparing ben/major_diameter (72c1f8f) with main (3fa4bfd)}

Summary

✅ 89 untouched benchmarks

[graphite-app]

Validation logic issue with optional parameters

There's a validation error in the major_radius matching logic. When both majorRadius and majorDiameter are None, the code returns an error:

(Some(_), Some(_)) | (None, None) => {
    return Err(KclError::new_type(KclErrorDetails::new(
        "Provide either `majorDiameter` or `majorRadius`, not both.".to_string(),
        vec![args.source_range],
    )));
}

However, according to the function signature, majorRadius is optional, and users can provide majorAxis instead. This validation breaks backward compatibility for code that uses majorAxis without specifying either radius parameter.

The validation should only reject cases where both diameter and radius are provided simultaneously, not when both are absent.

Suggested change

	let major_radius = match (major_radius, major_diameter) {
	(Some(_), Some(_)) | (None, None) => {
	return Err(KclError::new_type(KclErrorDetails::new(
	"Provide either `majorDiameter` or `majorRadius`, not both.".to_string(),
	vec![args.source_range],
	)));
	}
	(Some(major_radius), _) => Some(major_radius),
	(_, Some(major_diameter)) => Some(TyF64 {
	n: 0.5 * major_diameter.n,
	ty: major_diameter.ty,
	}),
	};
	let major_radius = match (major_radius, major_diameter) {
	(Some(_), Some(_)) => {
	return Err(KclError::new_type(KclErrorDetails::new(
	"Provide either `majorDiameter` or `majorRadius`, not both.".to_string(),
	vec![args.source_range],
	)));
	}
	(Some(major_radius), _) => Some(major_radius),
	(_, Some(major_diameter)) => Some(TyF64 {
	n: 0.5 * major_diameter.n,
	ty: major_diameter.ty,
	}),
	(None, None) => None,
	};

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.