Skip to content

Preserve type information from docstrings if no type annotation is present and parameter has default value. #576

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 16, 2025
Merged

Preserve type information from docstrings if no type annotation is present and parameter has default value. #576

merged 1 commit into from
Oct 16, 2025

Conversation

@christianaguilera-foundry
Copy link
Contributor

Fixes #574.
@gaborbernat gaborbernat marked this pull request as draft October 15, 2025 22:15
gaborbernat
gaborbernat reviewed Oct 15, 2025
View reviewed changes
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

Please add tests 👍
@christianaguilera-foundry
Copy link
Contributor Author

Please add tests 👍

Uhm... A test was already added. Do you mean "more tests"?

(Ruff is flagging that the function got to complex; normally, as an outsider, I'd refrain from refactoring, but I guess I have to.)
@gaborbernat
Copy link
Member

No th ones added are fine.
@christianaguilera-foundry christianaguilera-foundry force-pushed the default_values_and_type_information_in_docstrings branch from 08b0811 to 220cca6 Compare October 15, 2025 22:45
@christianaguilera-foundry christianaguilera-foundry marked this pull request as ready for review October 15, 2025 22:52
gaborbernat
gaborbernat reviewed Oct 15, 2025
View reviewed changes
@christianaguilera-foundry christianaguilera-foundry force-pushed the default_values_and_type_information_in_docstrings branch from 220cca6 to 98ec059 Compare October 15, 2025 23:14
gaborbernat
gaborbernat previously approved these changes Oct 16, 2025
View reviewed changes
@gaborbernat gaborbernat enabled auto-merge (squash) October 16, 2025 00:00
@christianaguilera-foundry
Copy link
Contributor Author

Apologies; my naive attempt at adding the unit tests without running locally didn't work well. I'm looking into it.
auto-merge was automatically disabled October 16, 2025 00:39

Head branch was pushed to by a user without write access

@christianaguilera-foundry christianaguilera-foundry force-pushed the default_values_and_type_information_in_docstrings branch from 98ec059 to 2fe2c44 Compare October 16, 2025 00:39
@christianaguilera-foundry
Copy link
Contributor Author

I think I got the unit tests right now.

For the records, I had some weird issues where, at some point, the unit tests would fail with some odd stale messages (switching to main; without my changes; would still fail). Then I had issues with "coverage" (going from 88% to 84%), even though the code I added is has 100% coverage (which means the overall coverage would increase). What I did to fix is was to remove the .tox directory and re-run; unsure how much of this was human error, or some known gotchas.
@gaborbernat gaborbernat merged commit bfbc005 into tox-dev:main Oct 16, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@christianaguilera-foundry @gaborbernat