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

Fix formatting type annotations for tuples with more than two arguments #5167

Merged
merged 2 commits into from
Jul 16, 2018

Conversation

mitya57
Copy link
Contributor

@mitya57 mitya57 commented Jul 13, 2018

Currently in Python 3.6, Sphinx formats this function:

def foo() -> Tuple[int, str, int]:
    pass

in the following way (which can be seen in the Travis log for the first commit in this PR):

() -> Tuple[[int, str], int]

This is because the tuple gets treated as a function, and the last int is treated like a return value.
This affects only format_annotation_old function. format_annotation_new which is used for Python 3.7 is not affected.

This pull request fixes this bug, and adds a test.

There is a hasattr(annotation, '__tuple_params__') clause below in that function. I am not sure for which Python version it is because all versions I tested (2.7, 3.5, 3.6, 3.7) do not have this attribute. Maybe it is for 3.4? Anyway I added an opposite condition (not hasattr) to be on the safe side.

@codecov
Copy link

codecov bot commented Jul 13, 2018

Codecov Report

Merging #5167 into 1.7 will increase coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##              1.7    #5167      +/-   ##
==========================================
+ Coverage   82.04%   82.04%   +<.01%     
==========================================
  Files         283      288       +5     
  Lines       37834    38161     +327     
  Branches     5860     5916      +56     
==========================================
+ Hits        31040    31309     +269     
- Misses       5486     5534      +48     
- Partials     1308     1318      +10
Impacted Files Coverage Δ
tests/test_util_inspect.py 90.12% <100%> (+0.08%) ⬆️
sphinx/util/inspect.py 73.42% <100%> (+0.25%) ⬆️
tests/typing_test_data.py 62.85% <50%> (-0.78%) ⬇️
sphinx/builders/html.py 82.76% <0%> (-0.12%) ⬇️
sphinx/search/__init__.py 95.11% <0%> (ø)
sphinx/apidoc.py 0% <0%> (ø)
sphinx/__init__.py 57.5% <0%> (ø)
sphinx/quickstart.py 0% <0%> (ø)
sphinx/errors.py 68.42% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fcbfa8...432f8c5. Read the comment docs.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

LGTM!

@tk0miya tk0miya added this to the 1.7.6 milestone Jul 16, 2018
@tk0miya tk0miya merged commit 9734bf5 into sphinx-doc:1.7 Jul 16, 2018
tk0miya added a commit that referenced this pull request Jul 16, 2018
@mitya57 mitya57 deleted the long-tuples branch August 31, 2018 11:08
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants