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

Migrate to mypy py3 mode #5760

Merged
merged 3 commits into from Dec 15, 2018

Conversation

@tk0miya
Copy link
Member

commented Dec 11, 2018

Feature or Bugfix

  • Refactoring

Purpose

  • Before moving to "type annotation" syntax in py3, we have to move to mypy-py3 mode.
  • To do that, we have to use docutils-stubs typing package to check our typing.
  • This try to migrate to mypy-py3 mode.
  • Thanks to @cocoatomo. He helps me to make docutils-stubs package and this PR.

@tk0miya tk0miya added the refactoring label Dec 11, 2018

@tk0miya tk0miya added this to the 2.0.0 milestone Dec 11, 2018

@tk0miya tk0miya force-pushed the tk0miya:migrate_to_mypy-py3 branch from 8b1c228 to 9b9eb83 Dec 12, 2018

@codecov

This comment has been minimized.

Copy link

commented Dec 12, 2018

Codecov Report

Merging #5760 into master will decrease coverage by 0.02%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5760      +/-   ##
==========================================
- Coverage   83.27%   83.24%   -0.03%     
==========================================
  Files         294      296       +2     
  Lines       39577    39614      +37     
  Branches     5878     5881       +3     
==========================================
+ Hits        32959    32978      +19     
- Misses       5257     5272      +15     
- Partials     1361     1364       +3
Impacted Files Coverage Δ
sphinx/cmd/build.py 0% <ø> (ø) ⬆️
setup.py 0% <ø> (ø) ⬆️
sphinx/__main__.py 0% <0%> (ø) ⬆️
sphinx/ext/graphviz.py 31.8% <0%> (ø) ⬆️
sphinx/builders/devhelp.py 42.1% <0%> (ø) ⬆️
sphinx/environment/__init__.py 76.29% <0%> (ø) ⬆️
sphinx/pycode/parser.py 94.68% <100%> (ø) ⬆️
sphinx/util/i18n.py 90.44% <100%> (ø) ⬆️
sphinx/domains/changeset.py 88.88% <100%> (ø) ⬆️
sphinx/builders/latex/__init__.py 78.26% <100%> (ø) ⬆️
... and 50 more

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 f8a2e7a...fe29753. Read the comment docs.

@tk0miya tk0miya force-pushed the tk0miya:migrate_to_mypy-py3 branch 2 times, most recently from db4d1df to 2fb31cb Dec 12, 2018

@@ -420,7 +420,7 @@ def depart_caption(self, node):

def visit_doctest_block(self, node):
# type: (nodes.doctest_block) -> None
self.visit_literal_block(node)
self.visit_literal_block(node) # type: ignore

This comment has been minimized.

Copy link
@tk0miya

tk0miya Dec 12, 2018

Author Member

I feel currrent annotations for visitor methods are too strict. I understand it is better to use concrete types to represent what type of node visitor methods accept. But it is too sensitive for authors of derived implementations.
Personally, using nodes.Element would better for writers. It is ambiguous, but easy to use.

@cocoatomo what do you think?

This comment has been minimized.

Copy link
@tk0miya

tk0miya Dec 14, 2018

Author Member

I talked with him directly, and we decided to use nodes.Element instead of concrete node subclasses because strictness harms developers.
refs: tk0miya/docutils-stubs#30

I'll update annotations of Sphinx side later.

@tk0miya tk0miya changed the title [WIP] Migrate to mypy py3 mode Migrate to mypy py3 mode Dec 12, 2018

@@ -6395,6 +6395,10 @@ def add_target_and_index(self, ast, sig, signode):
signode['first'] = (not self.names) # hmm, what is this about?
self.state.document.note_explicit_target(signode)

def get_index_text(self, name):
# type: (unicode) -> unicode
raise NotImplementedError()

This comment has been minimized.

Copy link
@tk0miya

tk0miya Dec 12, 2018

Author Member

Note: This PR contains a commit of #5755. So this should be merged after it.

This comment has been minimized.

Copy link
@tk0miya

tk0miya Dec 13, 2018

Author Member

merged and rebased.

@tk0miya tk0miya force-pushed the tk0miya:migrate_to_mypy-py3 branch from 1634ae2 to fe29753 Dec 14, 2018

@tk0miya tk0miya merged commit aaf0046 into sphinx-doc:master Dec 15, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tk0miya tk0miya deleted the tk0miya:migrate_to_mypy-py3 branch Dec 15, 2018

@tk0miya

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2018

We did it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.