Version 0.2.1#492
Merged
Merged
Conversation
…n works in this case
… changed trajectory in show_swept-file
updates: - [github.com/psf/black-pre-commit-mirror: 26.3.1 → 26.5.1](psf/black-pre-commit-mirror@26.3.1...26.5.1) - [github.com/astral-sh/ruff-pre-commit: v0.15.7 → v0.15.16](astral-sh/ruff-pre-commit@v0.15.7...v0.15.16) - [github.com/pre-commit/mirrors-clang-format: v22.1.1 → v22.1.5](pre-commit/mirrors-clang-format@v22.1.1...v22.1.5)
|
Warning Review limit reached
More reviews will be available in 53 minutes and 35 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (16)
WalkthroughAdds swept-spline creation and a FieldIntegrator API, refactors microstructure tile parameter/sensitivity handling with centralized helpers, enforces strict iterable-length checks across core/IO/tests, updates CI/tooling and examples, and expands test coverage for microtiles and integration. ChangesSwept Geometry Feature
Field Integration Framework
Microstructure Tile Parameter System Refactoring
Strict Iteration Safety and Supporting Changes
Build and Configuration Updates
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Member
Author
|
@copilot, please add a detailed changelog. If you find any glaring oversights, please also suggest changes. |
Contributor
Added
Also included a brief history stub for v0.2.0, v0.1.3, v0.1.2, and v0.1.1 with comparison links. Let me know if you'd like any section expanded or adjusted. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pyproject.toml (1)
27-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate
requires-pythonto match actual support.
requires-python = '>=3.8'is inconsistent with the dropped 3.9 support. The workflows now build wheels only for Python 3.10–3.14 (Lines 13, 38, 63, 88 inmain.yml) and test only on 3.10–3.14 (Line 21 intest_full.yml), and the classifiers list only 3.10–3.14. Users on Python 3.8 or 3.9 will encounter installation failures or be forced to build from source without any test coverage.📦 Proposed fix
-requires-python = '>=3.8' +requires-python = '>=3.10'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pyproject.toml` at line 27, Update the pyproject.toml requires-python metadata to match the actual supported Python versions used in CI and classifiers: change the requires-python value from '>=3.8' to '>=3.10' so it aligns with the workflows and classifiers (ensure the requires-python field is updated in pyproject.toml and that any related packaging metadata or classifiers remain consistent with supported versions 3.10–3.14).examples/show_microstructures.py (1)
367-378:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPass
separator_distanceto the secondshow()call too.Line 367 creates the forward microstructure with
separator_distance=0.4, but Line 370 visualizes it without that kwarg and withoutuse_saved=True. The “Corresponding Structure” plot can therefore stop matching themicrostructurecreated just above it.Suggested fix
_, showables = generator.show( closing_face="z", + separator_distance=0.4, center_expansion=1.3, title="Parametrized Microstructure", control_points=False, knots=True,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/show_microstructures.py` around lines 367 - 378, The visualization call to generator.show does not receive the same separator_distance used when creating the microstructure, so the "Corresponding Structure" plot can diverge; update the second call to generator.show (the one returning showables) to include separator_distance=0.4 (and optionally use_saved=True if you want to force showing the already-created microstructure rather than regenerating) so its parameters match generator.create.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/handle_markdown.py`:
- Around line 224-227: Remove the redundant else following the earlier continue
in the loop: unindent the new_path assignment and the content =
content.replace(...) call so they execute when the continue is not taken (i.e.,
delete the else and move the code out one indent level). This affects the block
that creates new_path (variable new_path) and replaces item[1] via
content.replace(item[1], str(new_path)) inside the same loop where a continue is
used.
- Around line 211-227: The file contains duplicate special-link processing:
get_special_links(...) and the subsequent for item in special_links loop that
mutates the full content (using content.replace(...)) duplicates the earlier
line-by-line handling that modifies each line before appending to content (the
code that inspects and updates the local variable line). Remove the full-content
pass (the special_links = get_special_links(content) block and its for loop) so
only the existing line-by-line special-link handling remains; ensure references
to get_special_links, special_links, content, and the per-line variable line are
updated/left consistent after removing the duplicate block.
In `@pyproject.toml`:
- Line 94: The pyproject.toml entry target-version = "py312" limits Ruff checks
to Python 3.12; change the Ruff configuration key target-version to the minimum
supported Python (e.g., set target-version = "py310") so Ruff validates
compatibility across the full supported range (3.10–3.14) — locate and update
the target-version value in the pyproject.toml Ruff settings.
In `@splinepy/helpme/create.py`:
- Line 224: In splinepy.helpme.create update the error message string
"Sorry,revolutions only implemented for 2D and 3D splines" to include the
missing space after the comma (i.e. "Sorry, revolutions only implemented for 2D
and 3D splines"); locate the literal in the create module (the error raised when
handling revolutions) and replace the string accordingly so the message reads
correctly.
- Line 109: Fix the typo in the error message string by removing the duplicated
word so it reads "Dimension Mismatch between extrusion vector and spline."
Replace the existing message "Dimension Mismatch between extrusion extrusion
vector and spline." (the exact string in create.py) with the corrected version
wherever it's used (e.g., in the error/exception raise or log call that emits
this message).
- Around line 643-667: The geometry_box sampling uses a fixed clamp for 2D
(sample_resolution ends up being 25) which ignores cross-section complexity;
update the logic around sample_resolution (check cross_section.para_dim) so
para_dim==2 computes per-axis resolution based on cross_section.control_points
(e.g., a function of control_points.shape[0] or para_dim-specific heuristic) or
expose a new parameter (e.g., geometry_box_resolution) to control sampling
density; update usage in the meshgrid/sampling block that builds sample_axes and
sample_queries so cs_min/cs_max/ cs_center reflect the finer adaptive resolution
when anchor="geometry_box"; add unit tests that create a refined/curved 2D
cross-section and assert geometry_box-derived cs_center matches a
higher-precision reference to prevent regressions.
In `@splinepy/microstructure/microstructure.py`:
- Around line 459-464: In Microstructure.create replace the assert-based type
checks for knot_span_wise and macro_sensitivities with explicit runtime type
validation: check if not isinstance(knot_span_wise, bool) and if not
isinstance(macro_sensitivities, bool) and raise a TypeError with clear messages
(fix the typo "macro_senstivities" to "macro_sensitivities"); update the error
text to explicitly state the expected type so invalid inputs fail immediately
rather than being skipped under python -O.
- Around line 278-280: The loop over enumerate(zip(self.tiling, ukvs,
strict=True)) can skip appending to additional_knots when tt <= n_current_spans,
causing misalignment consumed by _compute_tiling_prerequisites(); ensure
additional_knots stays positionally aligned with parametric axes by always
appending an entry per axis (append an empty list for axes that don't require
insertions). In other words, inside the loop that iterates (tt, ukv) (and when
checking n_current_spans and whether to insert knots), push either the computed
insertion list or [] into additional_knots so its length/order matches
self.param_dim and _compute_tiling_prerequisites() receives axis-aligned
entries.
In `@splinepy/microstructure/tiles/cross_3d_linear.py`:
- Around line 39-43: Update the docstring(s) that describe parameter bounds to
match the new dynamic logic in the _parameter_bounds property: change any
hardcoded “0.01 to 0.49” wording to state that the lower bound can be 0.0 and
the upper bound is capped at min(0.5, 0.5 / self._center_expansion), and mention
that the bounds are computed dynamically based on the _center_expansion
attribute; update the docstring on the _parameter_bounds property (and any
class-level or public API docstring referring to these bounds) accordingly.
In `@splinepy/microstructure/tiles/cube_void.py`:
- Around line 31-38: The parameter bounds in _parameter_bounds allow
combinations (e.g., r_x = r_yz = 1 with rotations) that, after rotations handled
in _process_input, push control points outside the unit cube; tighten the bounds
or add a validation step: either reduce radius bounds so rotated trilinear
control points always remain in [0,1]^3 (adjust entries in _parameter_bounds and
corresponding _default_parameter_value) or, inside _process_input (and before
create_tile assembles the spline), compute the transformed control-point extents
and raise/clip/adjust parameters if any coordinate lies outside [0,1]; also
apply the same fix for the analogous parameter block at lines referenced (the
second set around 105-108).
In `@splinepy/microstructure/tiles/hollow_octagon_extrude.py`:
- Around line 276-280: Before entering the branch ladder, explicitly validate
the closure parameter against the allowed set self._closure_directions and
reject unsupported values (including None) by raising a ValueError; locate the
method around the call to self._process_input (where parameters, n_derivatives,
derivatives are assigned) and add a check like "if closure not in
self._closure_directions: raise ValueError(...)" so the function fails fast
instead of falling through branches and later raising an UnboundLocalError when
patch variables are used.
- Around line 72-79: The closure branch in create_tile forwards **kwargs into
_closing_tile which doesn't accept extra keyword args, causing a TypeError; fix
by either updating _closing_tile signature to accept and ignore **kwargs (e.g.,
def _closing_tile(..., **kwargs):) or stop forwarding unknown extras from
create_tile by explicitly passing only parameters, parameter_sensitivities,
closure, and contact_length (i.e., remove **kwargs from the call or filter it
down to known keys) so open and closed tile paths accept the same call pattern;
reference create_tile and _closing_tile when making the change.
In `@splinepy/microstructure/tiles/hollow_octagon.py`:
- Around line 61-65: In create_tile, validate the closure parameter against the
class-level _closure_directions immediately after input processing (around the
call to self._process_input) and raise a ValueError for any unsupported value;
specifically check that closure is either None or contained in
self._closure_directions before computing right/top/left/bottom so unsupported
closure values do not fall through and cause an UnboundLocalError when variables
like right, top, etc. are later referenced.
In `@splinepy/microstructure/tiles/tile_base.py`:
- Around line 323-332: In _process_input: don't access
parameter_sensitivities.shape[2] before validation; call
check_param_derivatives(parameter_sensitivities) first and only then, if it's
not None, read parameter_sensitivities.shape[2] to set n_derivatives and prepare
derivatives; otherwise set n_derivatives = 0 and derivatives = None. This avoids
IndexError for malformed 2D/non-3D inputs and ensures
check_param_derivatives(...) runs before any indexing on
parameter_sensitivities.
- Around line 146-160: parameter_bounds currently returns the raw descriptor for
instance `@property` implementations because _raise_if_not_set_else_return
performs a class lookup; change parameter_bounds to first call
self._raise_if_not_set_else_return("_parameter_bounds") to validate presence and
then return getattr(self, "_parameter_bounds") so any instance property getter
is invoked. Also harden _process_input by calling check_param_derivatives()
before accessing parameter_sensitivities.shape[2] (and any other shape indexing)
so malformed inputs raise the intended validation errors instead of an
IndexError; reorder the checks in _process_input accordingly.
In `@tests/helpme/test_create.py`:
- Around line 464-556: The test is compensating for the cross-section normal
sign with a hardcoded "* -1" which depends on control-point ordering; instead,
in the test after computing normal_vector_custom_version from
calculate_cs_normal_vector(cs_cp, result_with_cus_normal_cps, rand), compute the
sign = np.sign(np.dot(rand_traj_tangent, normal_vector_custom_version)) and, if
sign is negative, flip the normal (multiply by -1) so the comparison uses a
consistently oriented normal; reference the swept() parameter
cross_section_normal and the local helper calculate_cs_normal_vector and
rand_traj_tangent when making this change, or alternatively add a short
clarifying comment explaining why the negation was needed if you prefer not to
change logic.
In `@tests/io/test_gismo.py`:
- Around line 163-169: tests/io/test_gismo.py contains parenthesized
multi-context with statements (e.g., in test_gismo_export_2D and other tests)
which use Python 3.9+ syntax and cause a parse error on Python 3.8; update each
occurrence (for example the with block near the test_gismo_export_2D function
and the blocks around ~216, ~269, ~579) to the Python 3.8-compatible form by
replacing the parenthesized "with (...)" with a single-line multi-context form
"with open(...) as tmp_read, open(...) as base_file:" (apply the same change for
all similar with blocks in test_gismo.py) or alternatively raise the project
minimum Python version in pyproject.toml if you prefer to require 3.9+.
In `@tests/io/test_mfem_export.py`:
- Around line 46-51: tests/io/test_mfem_export.py uses parenthesized context
managers (the "with (" block creating tmp_read and base_file) which requires
Python 3.10+, but pyproject.toml declares requires-python >=3.8; fix by either
updating pyproject.toml to requires-python = '>=3.10' to match the syntax or by
rewriting the with-block in test_mfem_export.py to be Python 3.8-compatible
(e.g., nest with statements or use contextlib.ExitStack to open tmpf and the
base file instead of the parenthesized "with (" form).
In `@tests/test_microtiles.py`:
- Around line 297-364: The test only perturbs whole “info-per-eval-point”
directions using _n_info_per_eval_point, missing individual flattened parameter
entries; change the loops and helper to iterate over each flattened parameter
index (use parameters.size or tile_creator._parameters_shape to compute total
entries) and for each index perturb parameters_perturbed.flat[idx] (or
parameters_perturbed.ravel()[idx]) when calling
derivative_finite_difference_evaluation, and build the corresponding
parameter_sensitivities with a single 1 at that flattened position (instead of a
single column at i_parameter) before calling tile_creator.create_tile so every
scalar parameter entry is tested; update references in
derivative_finite_difference_evaluation, the outer for-loop over i_parameter,
and the construction of parameter_sensitivities accordingly.
---
Outside diff comments:
In `@examples/show_microstructures.py`:
- Around line 367-378: The visualization call to generator.show does not receive
the same separator_distance used when creating the microstructure, so the
"Corresponding Structure" plot can diverge; update the second call to
generator.show (the one returning showables) to include separator_distance=0.4
(and optionally use_saved=True if you want to force showing the already-created
microstructure rather than regenerating) so its parameters match
generator.create.
In `@pyproject.toml`:
- Line 27: Update the pyproject.toml requires-python metadata to match the
actual supported Python versions used in CI and classifiers: change the
requires-python value from '>=3.8' to '>=3.10' so it aligns with the workflows
and classifiers (ensure the requires-python field is updated in pyproject.toml
and that any related packaging metadata or classifiers remain consistent with
supported versions 3.10–3.14).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6699b1f-f5fa-4ffb-9bc9-23006919e9e3
⛔ Files ignored due to path filters (1)
docs/source/_static/show_swept.pngis excluded by!**/*.png
📒 Files selected for processing (56)
.github/workflows/main.yml.github/workflows/test_full.yml.pre-commit-config.yamlREADME.mddocs/source/handle_markdown.pyexamples/iga/collocation_stokes_problem_sparse.pyexamples/iga/galerkin_laplace_problem_field_integrator.pyexamples/show_microstructures.pyexamples/show_readme.pyexamples/show_swept.pypyproject.tomlsplinepy/bspline.pysplinepy/helpme/check.pysplinepy/helpme/create.pysplinepy/helpme/integrate.pysplinepy/io/gismo.pysplinepy/io/irit.pysplinepy/io/mfem.pysplinepy/io/svg.pysplinepy/microstructure/microstructure.pysplinepy/microstructure/tiles/__init__.pysplinepy/microstructure/tiles/armadillo.pysplinepy/microstructure/tiles/chi.pysplinepy/microstructure/tiles/cross_2d.pysplinepy/microstructure/tiles/cross_3d.pysplinepy/microstructure/tiles/cross_3d_linear.pysplinepy/microstructure/tiles/cube_void.pysplinepy/microstructure/tiles/double_lattice.pysplinepy/microstructure/tiles/ellips_v_oid.pysplinepy/microstructure/tiles/hollow_cube.pysplinepy/microstructure/tiles/hollow_octagon.pysplinepy/microstructure/tiles/hollow_octagon_extrude.pysplinepy/microstructure/tiles/inverse_cross_3d.pysplinepy/microstructure/tiles/snappy.pysplinepy/microstructure/tiles/tile_base.pysplinepy/utils/data.pytests/conftest.pytests/data/mfem_cartesian_2d.meshtests/data/mfem_cartesian_3d.meshtests/helpme/test_create.pytests/helpme/test_integrate.pytests/io/test_cats.pytests/io/test_gismo.pytests/io/test_iges.pytests/io/test_irit.pytests/io/test_json.pytests/io/test_mfem_export.pytests/io/test_npz.pytests/test_bezier_extraction.pytests/test_bezier_operations.pytests/test_knot_vectors.pytests/test_kv_manipulation.pytests/test_microstructure.pytests/test_microtiles.pytests/test_multipatch.pytests/test_normalize_knot_vectors.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 8: Replace the unconventional header "## [Unreleased] - v0.2.1" with "##
[Unreleased]" for now; when cutting the release, rename that header to "##
[0.2.1] - YYYY-MM-DD" (using the actual release date) and update the comparison
link currently using "v0.2.0...HEAD" to "v0.2.0...v0.2.1" so the changelog
follows Keep a Changelog conventions; look for the header string "##
[Unreleased] - v0.2.1" and the comparison link fragment "v0.2.0...HEAD" to make
these edits.
- Line 111: In CHANGELOG.md update the GitHub repo reference on the bspline.py
entry: replace the string "tataratat/splinepy" with "isosuite/splinepy" in the
line containing **`bspline.py`** so it matches the other comparison links (e.g.,
those at lines ~153-157) and keeps repo references consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
… why?) lower reference python version for ruff
Member
|
Nice! Don't forget to bump the version |
Member
Author
Already in a local commit. I am still working on some of the issues rabbit has unearthed. |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Prepares the v0.2.1 release with swept surface/solid generation, IGA assembly helpers, microstructure tile improvements, and a detailed project changelog.
Addressed issues
Release Notes
New Features
splinepy.helpme.create.swept()Improvements
check_params)TileBaseDocumentation
Bug Fixes
Chores
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
Chores