Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first docstrings for linear model #691

Merged
merged 7 commits into from
Jul 14, 2024

Conversation

david26694
Copy link
Contributor

@david26694 david26694 commented Jul 13, 2024

This adds docstrings for all the linear_model in #596. It's just basic fit-predict usage, let me know if you'd like something more detailed @FBruzzesi

@david26694 david26694 marked this pull request as ready for review July 13, 2024 13:10
Copy link
Collaborator

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

I have a minor concern for DemographicParity and EqualOpportunity: the code snippet "overflows" the screen, even on a large one - (see screenshot). I left comments for how we could format the code for DemographicParityClassifier , and the same should apply to EqualOpportunityClassifier.

We may consider linting and formatting docstring code snippet programmatically as well in the future I guess.

image

Comment on lines +180 to +189
# The weights sum up to 1
assert np.isclose(pwr.coef_.sum(), 1)

X_test = np.array([[5, 6], [6, 7]])

# The prediction is positive (all weights are positive, and features are positive)
assert all(pwr.predict(X_test) > 0)

# The weights are positive
assert all(pwr.coef_ > -1e-8)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like these "behavior"-like assertions :)

sklego/linear_model.py Outdated Show resolved Hide resolved
sklego/linear_model.py Outdated Show resolved Hide resolved
david26694 and others added 4 commits July 14, 2024 16:56
Co-authored-by: Francesco Bruzzesi <42817048+FBruzzesi@users.noreply.github.com>
Co-authored-by: Francesco Bruzzesi <42817048+FBruzzesi@users.noreply.github.com>
@david26694 david26694 requested a review from FBruzzesi July 14, 2024 15:39
Copy link
Collaborator

@FBruzzesi FBruzzesi left a comment

Choose a reason for hiding this comment

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

Thanks @david26694 🙌🏼🚀

@FBruzzesi FBruzzesi merged commit 0cfc545 into koaning:main Jul 14, 2024
17 checks passed
@FBruzzesi FBruzzesi mentioned this pull request Jul 14, 2024
33 tasks
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