Skip to content

Conversation

samestep
Copy link
Contributor

This PR addresses a two-year-old TODO in test/test_type_hints.py by replacing most of the body of our custom get_examples_from_docstring function with a function from Python's built-in doctest.DocTestParser class. This mostly made the parser more strict, catching a few errors in existing doctests:

  • missing ... in multiline statements
  • missing space after >>>
  • unmatched closing parenthesis

Also, as shown by the resulting diff of the untracked test/generated_type_hints_smoketest.py file (also linked from the test plan below), this introduces a few incidental changes as well:

Test plan:

Checkout the base commit, typecheck the doctests, and save the generated file:

$ python test/test_type_hints.py TestTypeHints.test_doc_examples
$ cp test/generated_type_hints_smoketest.py /tmp

Then checkout this PR, do the same thing, and compare:

$ python test/test_type_hints.py TestTypeHints.test_doc_examples
$ git diff --no-index {/tmp,test}/generated_type_hints_smoketest.py

The test should succeed, and the diff should match this paste.

@samestep samestep requested a review from a team January 15, 2021 17:47
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

looks good.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@samestep has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@codecov
Copy link

codecov bot commented Jan 20, 2021

Codecov Report

Merging #50596 (1e1159f) into master (1f5c3b3) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #50596      +/-   ##
==========================================
- Coverage   81.07%   81.06%   -0.01%     
==========================================
  Files        1915     1915              
  Lines      208621   208621              
==========================================
- Hits       169139   169126      -13     
- Misses      39482    39495      +13     

@facebook-github-bot
Copy link
Contributor

@samestep merged this pull request in c147aa3.

@samestep samestep deleted the use-doctest-directly branch April 21, 2021 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants