Skip to content

fix: Three bugs in WeakSINDy/EmptyLibrary from code review#682

Closed
Copilot wants to merge 2 commits intoweakfrom
copilot/sub-pr-678
Closed

fix: Three bugs in WeakSINDy/EmptyLibrary from code review#682
Copilot wants to merge 2 commits intoweakfrom
copilot/sub-pr-678

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 27, 2026

Code review of the WeakSINDy PR uncovered three bugs.

Changes

  • EmptyLibrary.transform shape error (feature_library/base.py): np.empty((x.shape[:-1], 0)) wraps the shape in a nested tuple, causing a TypeError at runtime. Fixed to np.empty((*x.shape[:-1], 0)).

  • WeakSINDy.set_params missing return value (_weak.py): Violated the sklearn convention of returning self, breaking method chaining (model.set_params(...).fit(...)). Added return self.

  • Variable shadowing in _plan_weak_form (_weak.py): for lib in lib.libraries: shadowed the function parameter, with a nested for lib in lib.libraries generator compounding the issue. Renamed loop variables to sublib/sl to eliminate the shadowing.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…le shadowing in _plan_weak_form

Co-authored-by: Jacob-Stevens-Haas <37048747+Jacob-Stevens-Haas@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor weak form SINDy to separate SINDy class fix: Three bugs in WeakSINDy/EmptyLibrary from code review Feb 27, 2026
@Jacob-Stevens-Haas
Copy link
Copy Markdown
Member

This works well, but I changed some names and will push directly to _weak.

Is there a way to change your commit behavior? I didn't like that you had a commit message title exceeding 72 characters and that you created an empty "Initial Commit" commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants