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

Add defaults #191

Merged
merged 12 commits into from
Jan 5, 2022
Merged

Add defaults #191

merged 12 commits into from
Jan 5, 2022

Conversation

flying-sheep
Copy link
Contributor

@flying-sheep flying-sheep commented Jan 4, 2022

Supersedes #103, fixes #48

In the previous PR, I added them to types, this time I’m adding it to the string. I think both have arguments pro and contra. Should it be configurable?

@gaborbernat gaborbernat marked this pull request as draft January 4, 2022 09:44
@gaborbernat
Copy link
Member

I'd like to have it by default off. Otherwise, I'm happy to also allow it to be configurable where it's added.

@flying-sheep
Copy link
Contributor Author

flying-sheep commented Jan 4, 2022

OK, I need help, I have no idea what’s going on with those "s and *s.

Why does only my test case fail on Python 3.9 even though I copied the setup from test_sphinx_output_future_annotations?

@flying-sheep flying-sheep marked this pull request as ready for review January 4, 2022 11:29
@gaborbernat gaborbernat marked this pull request as draft January 4, 2022 12:12
@gaborbernat
Copy link
Member

OK, I need help, I have no idea what’s going on with those "s and *s.

Why does only my test case fail on Python 3.9 even though I copied the setup from test_sphinx_output_future_annotations?

That's half the work in this project, to figure out the differences between Python versions and make it work everywhere. I'll have a look at some point, but no idea when I'll have time for this.

@flying-sheep
Copy link
Contributor Author

flying-sheep commented Jan 5, 2022

Ugh, I figured it out. This will bite you in the future, I hope you find a way to make it less flaky. I filed #194 to explore this.

This PR is now ready! Changelog, readme, code, tests!

@flying-sheep flying-sheep marked this pull request as ready for review January 5, 2022 09:14
@gaborbernat
Copy link
Member

Ugh, I figured it out. This will bite you in the future, I hope you find a way to make it less flaky.

I filed #194 to explore this. But this PR is now ready!

Sorry, I'm not open to a fallback issue and working around the test. We should figure out here and now why this is the case and avoid it.

@flying-sheep
Copy link
Contributor Author

flying-sheep commented Jan 5, 2022

I’m avoiding it by not using that syntax, in the same way your main test does. This PR is completely independent of this problem now, I just happened to run into it while developing this PR.

I’m happy to help, but please don‘t drag this unrelated PR into it.

src/sphinx_autodoc_typehints/__init__.py Outdated Show resolved Hide resolved
src/sphinx_autodoc_typehints/__init__.py Show resolved Hide resolved
src/sphinx_autodoc_typehints/__init__.py Outdated Show resolved Hide resolved
@gaborbernat gaborbernat merged commit 8dc8d94 into tox-dev:main Jan 5, 2022
@flying-sheep flying-sheep deleted the defaults branch January 5, 2022 11:50
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.

Also annotate parameter defaults.
2 participants