Skip to content

Conversation

christianaguilera-foundry
Copy link
Contributor

Fixes #574.

@gaborbernat gaborbernat marked this pull request as draft October 15, 2025 22:15
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
@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
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parameters with a default value lose their type information from docstrings

2 participants