Skip to content

Conversation

damskii9992
Copy link
Contributor

Small PR to fix issue #133

@damskii9992 damskii9992 added [scope] bug Bug report or fix (major.minor.PATCH) [maintainer] good first issue Good entry-level issue for newcomers [priority] highest Urgent. Needs attention ASAP labels Sep 19, 2025
@damskii9992 damskii9992 linked an issue Sep 19, 2025 that may be closed by this pull request
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not contain a valid label. Please add one of the following labels: ['chore', 'fix', 'bugfix', 'bug', 'enhancement', 'feature', 'dependencies', 'documentation']

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not contain a valid label. Please add one of the following labels: ['chore', 'fix', 'bugfix', 'bug', 'enhancement', 'feature', 'dependencies', 'documentation']

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not contain a valid label. Please add one of the following labels: ['chore', 'fix', 'bugfix', 'bug', 'enhancement', 'feature', 'dependencies', 'documentation']

@damskii9992 damskii9992 added the fix PR label label Sep 19, 2025
Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the only issue is to potentially add another check

raise ValueError('Weights cannot be NaN or infinite.')

if (weights <= 0).any():
raise ValueError('Weights must be strictly positive and non-zero.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it would be prudent to also check the size, which needs to be the same as x

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was also no check on y. I added checks to both and updated the unit tests.

@damskii9992 damskii9992 merged commit c442841 into develop Sep 19, 2025
28 checks passed
@damskii9992 damskii9992 deleted the issue133_fitting_weights branch September 19, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix PR label [maintainer] good first issue Good entry-level issue for newcomers [priority] highest Urgent. Needs attention ASAP [scope] bug Bug report or fix (major.minor.PATCH)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default fitting weights
2 participants