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

⬆️ UPGRADE: Drop support for EOL Python 3.6 #194

Merged
merged 5 commits into from
Feb 18, 2022

Conversation

hukkin
Copy link
Member

@hukkin hukkin commented Jan 25, 2022

  • Drop Python 3.6 support
  • Remove Python 3.6 from CI
  • Modernize type annotations
  • Add an "all good" CI job

Closes #193

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #194 (dd46291) into master (bf8ca33) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #194      +/-   ##
==========================================
- Coverage   96.17%   96.14%   -0.04%     
==========================================
  Files          61       61              
  Lines        3270     3244      -26     
==========================================
- Hits         3145     3119      -26     
  Misses        125      125              
Flag Coverage Δ
pytests 96.14% <100.00%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
markdown_it/cli/parse.py 88.23% <100.00%> (+0.23%) ⬆️
markdown_it/common/normalize_url.py 84.37% <100.00%> (+0.50%) ⬆️
markdown_it/main.py 89.31% <100.00%> (+0.08%) ⬆️
markdown_it/parser_block.py 93.47% <100.00%> (ø)
markdown_it/parser_core.py 100.00% <100.00%> (ø)
markdown_it/parser_inline.py 96.72% <100.00%> (ø)
markdown_it/renderer.py 99.13% <100.00%> (+<0.01%) ⬆️
markdown_it/ruler.py 89.65% <100.00%> (ø)
markdown_it/rules_block/blockquote.py 100.00% <100.00%> (ø)
markdown_it/rules_block/heading.py 100.00% <100.00%> (ø)
... and 16 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 bf8ca33...dd46291. Read the comment docs.

@hukkin
Copy link
Member Author

hukkin commented Jan 25, 2022

I added an "all good" CI job.

Currently we have "tests (3.6)" and "tests (pypy-3.6)" set as Required jobs in GitHub but they no longer exist in the CI matrix. It can be annoying to update the set of required jobs whenever dropping or adding python versions so this is a workaround: we only require the "allgood" job to pass, and the "allgood" job only passes if all existing test jobs and pre-commit job passes.

I can remove this if it's not something you like.

setup.cfg Outdated Show resolved Hide resolved
@chrisjsewell
Copy link
Member

Cheers, I'll probably have a look at these next week now

@@ -15,7 +17,7 @@
version_str = "markdown-it-py [version {}]".format(__version__)


def main(args: Optional[Sequence[str]] = None) -> int:
def main(args: Sequence[str] | None = None) -> int:
Copy link
Member

Choose a reason for hiding this comment

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

Oh I didn't think type unions were supported yet: https://www.python.org/dev/peps/pep-0604/
But I guess if it works it works? Or is there any issue with type evaluation pre python 3.10?

Copy link
Member

Choose a reason for hiding this comment

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

dammit, made this comment ages ago, but didn't realise it was pending

Copy link
Member Author

@hukkin hukkin Feb 10, 2022

Choose a reason for hiding this comment

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

This works perfectly!

It would be a syntax error (pre Python 3.10) but from __future__ import annotations (first available in Python 3.7) makes it not error.

And mypy doesn't care what our minimum supported Python version is.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure how mypy does that 🤷 (are you sure it actually picks them up, and not silently fails)

On python 3.10 you can do:

In [1]: def bar() -> int | str:
   ...:     pass
In [2]: from typing import get_type_hints
In [3]: get_type_hints(bar)
Out[3]: {'return': int | str}

but on python 3.8, you get:

In [1]: from __future__ import annotations
In [2]: from typing import get_type_hints
In [3]: def bar() -> int | str:
   ...:     pass
   ...: 
In [4]: get_type_hints(bar)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-348bd5169782> in <module>
----> 1 get_type_hints(bar)

~/Documents/GitHub/aiida_core_develop/.tox/py38-pre-commit/lib/python3.8/typing.py in get_type_hints(obj, globalns, localns)
   1262         if isinstance(value, str):
   1263             value = ForwardRef(value)
-> 1264         value = _eval_type(value, globalns, localns)
   1265         if name in defaults and defaults[name] is None:
   1266             value = Optional[value]

~/Documents/GitHub/aiida_core_develop/.tox/py38-pre-commit/lib/python3.8/typing.py in _eval_type(t, globalns, localns)
    268     """
    269     if isinstance(t, ForwardRef):
--> 270         return t._evaluate(globalns, localns)
    271     if isinstance(t, _GenericAlias):
    272         ev_args = tuple(_eval_type(a, globalns, localns) for a in t.__args__)

~/Documents/GitHub/aiida_core_develop/.tox/py38-pre-commit/lib/python3.8/typing.py in _evaluate(self, globalns, localns)
    516                 localns = globalns
    517             self.__forward_value__ = _type_check(
--> 518                 eval(self.__forward_code__, globalns, localns),
    519                 "Forward references must evaluate to types.",
    520                 is_argument=self.__forward_is_argument__)

<string> in <module>

TypeError: unsupported operand type(s) for |: 'type' and 'type'

Copy link
Member Author

@hukkin hukkin Feb 10, 2022

Choose a reason for hiding this comment

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

I'm sure it works. Mypy is a static type checker. It doesn't run the code it type checks.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't run the code it type checks

Not sure about sphinx autodoc though

Copy link
Member

Choose a reason for hiding this comment

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

Ah think it just uses:

In [13]: import inspect
In [16]: def bar() -> int | str:
    ...:     pass
    ...: 
In [17]: inspect.signature(bar).return_annotation
Out[17]: 'int | str'

and never evaluates, so should be fine

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought sphinx-autodoc doesn't do anything with type annotations unless sphinx-autodoc-typehints is in use (it isn't)?

And the plugin certainly supports what we do in this PR: tox-dev/sphinx-autodoc-typehints#184

Copy link
Member

Choose a reason for hiding this comment

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

Yeh exactly, looks all good 👍; I just wanted to make sure I understood the implications of using future annotations, before rolling them out everywhere

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Cheers!

@chrisjsewell chrisjsewell changed the title 🔧 MAINTAIN: Drop support for EOL Python 3.6 ⬆️ UPGRADE: Drop support for EOL Python 3.6 Feb 18, 2022
@chrisjsewell chrisjsewell merged commit 32097fb into executablebooks:master Feb 18, 2022
@hukkin hukkin deleted the drop-py36 branch February 18, 2022 20:13
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.

Drop support for end-of-life Python 3.6
3 participants