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

docs: explain optional boolean args in from_native #1421

Merged
merged 9 commits into from
Nov 22, 2024

Conversation

MarcoGorelli
Copy link
Member

closes #1420

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

  • Related issue #<issue number>
  • Closes #<issue number>

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 22, 2024
@FBruzzesi
Copy link
Member

Is there a reason to have Nones in the first place? From a user point of view, I think it would be more clear to have boolean defaults πŸ€”

@MarcoGorelli
Copy link
Member Author

For some of these, yes - good call. That would still be backwards-compatible. For others (e.g. strict / pass_through) we explicitly check is None to ensure that users don't pass both together, so at least until stable.v2 I think they need to stay as bool | None

The other one which we can't change is allow_series, because we check for the invalid combination allow_series=False, series_only=True

Agree on simplifying where possible though, thanks πŸ‘

@MarcoGorelli MarcoGorelli marked this pull request as ready for review November 22, 2024 10:41
@MarcoGorelli MarcoGorelli marked this pull request as draft November 22, 2024 10:45
Comment on lines +4 to +5
# Hopefully temporary until https://github.com/mkdocstrings/mkdocstrings/issues/716
git+https://github.com/MarcoGorelli/griffe.git@no-overloads
Copy link
Member Author

Choose a reason for hiding this comment

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

temporary (hopefully 🀞 ) workaround for mkdocstrings/mkdocstrings#716

@MarcoGorelli MarcoGorelli marked this pull request as ready for review November 22, 2024 14:52
@MarcoGorelli
Copy link
Member Author

thanks for your review, and thanks @emilykl for helping us improve the docs, really appreciate this and the other suggestions you made!

@MarcoGorelli MarcoGorelli merged commit 6946e26 into narwhals-dev:main Nov 22, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Doc]: Clarify behavior of boolean arguments to from_native()
2 participants