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 ci: pin linux, pin flake8, ignore type error, fix mypy #308

Merged
merged 4 commits into from
Jul 19, 2023

Conversation

julianlocke
Copy link
Contributor

@julianlocke julianlocke commented Jul 18, 2023

Removes python 2 support.
Ignores a flake8 type error caused by a conditional import of two (similar for our purposes) types.
Pins flake8 to a version <6 as versions past that no longer parse type annotations. Without the type annotations, we encounter "imported but unused" errors.
Removes now-unrecognized show_none_errors mypy setting. Apparently strict_optional enforces this anyway.
Pins linux to ubuntu-20.04, as versions past that no longer ship with python 3.6.

Checklist

General Contributing

  • Have you read the Code of Conduct and signed the CLA?

Is This a Code Change?

  • Non-code related change (markdown/git settings etc)
  • Code Change
  • Example/Test Code Change

Validation

  • Have you ran tox?
  • Do the tests pass?

@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #308 (681bd5e) into main (199029a) will increase coverage by 0.00%.
The diff coverage is 42.85%.

❗ Current head 681bd5e differs from pull request most recent head 68afdcc. Consider uploading reports for the commit 68afdcc to get more accurate results

@@           Coverage Diff           @@
##             main     #308   +/-   ##
=======================================
  Coverage   52.02%   52.02%           
=======================================
  Files          37       37           
  Lines        8462     8451   -11     
  Branches     1810     1810           
=======================================
- Hits         4402     4397    -5     
+ Misses       3743     3737    -6     
  Partials      317      317           
Flag Coverage Δ
unit 52.02% <42.85%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
stone/backends/js_types.py 0.00% <0.00%> (ø)
stone/backends/obj_c_client.py 0.00% <0.00%> (ø)
stone/backends/obj_c_types.py 0.00% <0.00%> (ø)
stone/backends/swift_client.py 0.00% <0.00%> (ø)
stone/backends/swift_types.py 0.00% <0.00%> (ø)
stone/cli.py 0.00% <0.00%> (ø)
stone/frontend/ir_generator.py 80.43% <50.00%> (-0.03%) ⬇️
stone/ir/data_types.py 80.57% <50.00%> (ø)
stone/backends/js_client.py 80.48% <100.00%> (-0.24%) ⬇️
stone/backends/python_client.py 46.46% <100.00%> (-0.20%) ⬇️
... and 2 more

Copy link
Contributor

@devPalacio devPalacio left a comment

Choose a reason for hiding this comment

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

It's beautiful, good riddance python2

@devPalacio
Copy link
Contributor

Maybe we should update the readme to reflect that Python 3 is required? Is that just a given since we're 3 years past EOL?

@johnlarkin1
Copy link
Contributor

Maybe we should update the readme to reflect that Python 3 is required? Is that just a given since we're 3 years past EOL?

I like this idea. I think it's probably about time.

Copy link
Contributor

@johnlarkin1 johnlarkin1 left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for doing this.

Comment on lines -8 to -14
# Hack to get around some of Python 2's standard library modules that
# accept ascii-encodable unicode literals in lieu of strs, but where
# actually passing such literals results in errors with mypy --py2. See
# <https://github.com/python/typeshed/issues/756> and
# <https://github.com/python/mypy/issues/2536>.
import importlib
argparse = importlib.import_module(str('argparse')) # type: typing.Any
Copy link
Contributor

Choose a reason for hiding this comment

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

😍 love to see this.

@@ -9,19 +9,13 @@
try:
from inspect import getfullargspec as get_args
except ImportError:
from inspect import getargspec as get_args
from inspect import getargspec as get_args # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks

@@ -27,7 +27,7 @@
from test.backend_test_util import _mock_output


def _make_backend(target_folder_path, template_path, custom_args=None):
def _make_backend(target_folder_path, template_path, custom_args=None): # type: ignore
# type: (typing.Text, typing.Text, typing.List) -> TSDTypesBackend
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for the sake of this PR, but we should be able to update these to modern type hint syntax right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll run this one day: https://github.com/ilevkivskyi/com2ann

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we'll be able to unpin flake8

@julianlocke julianlocke merged commit 82ee5e3 into main Jul 19, 2023
@julianlocke julianlocke deleted the fix-ci-checks branch July 19, 2023 16:01
julianlocke added a commit that referenced this pull request Jul 20, 2023
* Fix ci: pin linux, pin flake8, ignore type error, fix mypy (#308)

* Replace uses of deprecated imp with importlib (#307)

* Update cli.py to work with java sdk (#310)

---------

Co-authored-by: Jay <[email protected]>
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.

3 participants