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

🔧 MAINTAIN: Add mypy type-checking #64

Merged
merged 13 commits into from
Dec 14, 2020

Conversation

hukkin
Copy link
Contributor

@hukkin hukkin commented Oct 21, 2020

Closes #19

@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #64 (515fee0) into master (3a5bdcc) will increase coverage by 0.06%.
The diff coverage is 96.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #64      +/-   ##
==========================================
+ Coverage   95.69%   95.75%   +0.06%     
==========================================
  Files          78       78              
  Lines        3970     3984      +14     
==========================================
+ Hits         3799     3815      +16     
+ Misses        171      169       -2     
Flag Coverage Δ
pytests 95.75% <96.05%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
markdown_it/common/utils.py 74.35% <0.00%> (+2.75%) ⬆️
markdown_it/token.py 91.76% <77.77%> (-1.01%) ⬇️
markdown_it/common/entities.py 92.30% <100.00%> (ø)
markdown_it/common/normalize_url.py 95.23% <100.00%> (ø)
markdown_it/extensions/anchors/index.py 100.00% <100.00%> (ø)
markdown_it/extensions/container/index.py 97.87% <100.00%> (+0.02%) ⬆️
markdown_it/extensions/deflist/index.py 94.96% <100.00%> (ø)
markdown_it/extensions/footnote/index.py 95.16% <100.00%> (+0.01%) ⬆️
markdown_it/extensions/tasklists/__init__.py 75.00% <100.00%> (+0.35%) ⬆️
markdown_it/main.py 90.98% <100.00%> (ø)
... and 14 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 3a5bdcc...515fee0. Read the comment docs.

raise NotImplementedError
# if "\\" in string:
# return string
# return string.replace(UNESCAPE_MD_RE, "$1")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I simply commented this function out, because str.replace() takes strings as input, not re.Patterns, so this is probably broken.

@chrisjsewell
Copy link
Member

I will get round to looking at this eventually! Just want to make sure I look thoroughly, since its touching core code

@hukkin
Copy link
Contributor Author

hukkin commented Oct 31, 2020

I rebased on master, and added one more commit that adds no_implicit_optional = True mypy configuration. This is to conform to updated PEP 484. Eventually this will be default mypy configuration too.

@chrisjsewell
Copy link
Member

👍

this will be default mypy configuration too.

TBH it would be so much nicer and concise if you could use the typescript ? notation:

def func(a: Employee? = None):
  pass

@hukkin
Copy link
Contributor Author

hukkin commented Oct 31, 2020

I think I've seen some discussion around that Optional syntax somewhere. Python typing is moving forward at quite a rapid pace so maybe one day we'll see that. In the next Python release we already get union types with TypeX | TypeY which I really like :)

@chrisjsewell
Copy link
Member

Oh thats nice 😄 Yeh for typing, I think they should just copy everything from typescript, which they are basically gradually doing

@hukkin
Copy link
Contributor Author

hukkin commented Oct 31, 2020

Haha yeh seems so. Btw. the new union syntax obviously lets us do int | None which is more concise than Optional[int] and requires no import from typing, so partially solves the problem.

@hukkin hukkin changed the title Add mypy. Fix typing errors 🔧 MAINTAIN: Add mypy to CI. Fix typing errors Dec 13, 2020
@chrisjsewell chrisjsewell mentioned this pull request Dec 14, 2020
@chrisjsewell chrisjsewell changed the title 🔧 MAINTAIN: Add mypy to CI. Fix typing errors 🔧 MAINTAIN: Add mypy type-checking Dec 14, 2020
@chrisjsewell chrisjsewell merged commit b16ee04 into executablebooks:master Dec 14, 2020
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.

MyPy Typechecking
3 participants