Skip to content

Drop Python 3.5 support #83

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

Merged
merged 2 commits into from
Apr 26, 2021
Merged

Drop Python 3.5 support #83

merged 2 commits into from
Apr 26, 2021

Conversation

joshthoward
Copy link
Member

@joshthoward joshthoward commented Apr 4, 2021

This PR includes the following updates:

  1. Dropping Python 3.5 support
  2. Removing Python 2 type annotations

@hashhar
Copy link
Member

hashhar commented Apr 5, 2021

For removing Python 3.5 you also need to update the setup.py python_requires AFAIK.

@joshthoward joshthoward force-pushed the jh/updates branch 4 times, most recently from f997cb8 to 7d02557 Compare April 5, 2021 14:42
@joshthoward joshthoward requested a review from hashhar April 5, 2021 14:50
@joshthoward
Copy link
Member Author

@hashhar disregard Mac OS and Windows support... I need to think about whether we want to run docker on each of those platforms or if we should have another service up and running for integration testing.

Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

I'm not so sure about the f-string changes (others may disagree) - they are readable in some cases but not in others. Also, can IDEs/static analysis tools catch cases where a f-string references a non-existent/mis-spelled variable?

e.g.

return "MAP({}, {})".format(
    self._format_prepared_param(keys),
    self._format_prepared_param(values)
)

vs

return f"MAP({self._format_prepared_param(keys)},
    {self._format_prepared_param(values)})"
```.

Also, are we using mypy in CI to actually verify the type annotations we have?
What end-user behaviour is for people who try to install the package for an unsupported version of Python?

Copy link
Member

@hashhar hashhar 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.

@joshthoward What does a user trying to install on Python3.5 see? Do get the older version? Or does it silently fail?

@hashhar hashhar changed the title Updates to CI Drop Python 3.5 support Apr 20, 2021
@joshthoward
Copy link
Member Author

@hashhar It complains if you try to use Python 3.5. See below:

% pyenv virtualenv 3.5.10 test
Requirement already satisfied: setuptools in /Users/joshthoward/.pyenv/versions/3.5.10/envs/test/lib/python3.5/site-packages
Requirement already satisfied: pip in /Users/joshthoward/.pyenv/versions/3.5.10/envs/test/lib/python3.5/site-packages
% pyenv activate test
% pip install .
Processing /Users/joshthoward/Documents/GitHub/trino-python-client
trino requires Python '>=3.6' but the running Python is 3.5.10

@hashhar
Copy link
Member

hashhar commented Apr 20, 2021

Awesome! It's a useful error message. I wondered if it was something cryptic and would make the 3.5 users confused about why installs broke.

@findepi findepi merged commit 1c7149f into trinodb:master Apr 26, 2021
@findepi findepi mentioned this pull request Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants