Skip to content

mypy linting and type checking#26

Open
aaronsteers wants to merge 27 commits intotransferwise:masterfrom
aaronsteers:feature/mypy-typing-checks
Open

mypy linting and type checking#26
aaronsteers wants to merge 27 commits intotransferwise:masterfrom
aaronsteers:feature/mypy-typing-checks

Conversation

@aaronsteers
Copy link
Copy Markdown

@aaronsteers aaronsteers commented May 14, 2021

Adding mypy checks and type hints.

I intentionally used Any on types if I was not sure of the expected type myself.

Tests are passing locally but for some reason I'm not getting them to run in CI. I'll check on that and post back.

@aaronsteers aaronsteers marked this pull request as ready for review May 15, 2021 00:03
@aaronsteers aaronsteers mentioned this pull request May 15, 2021
@aaronsteers aaronsteers changed the title Draft: mypy linting and type checking mypy linting and type checking May 20, 2021
@aaronsteers
Copy link
Copy Markdown
Author

aaronsteers commented May 20, 2021

@Samira-El and @koszti - This is ready for review! 🚀 😄

image

Link to CI jobs on my fork:
https://github.com/aaronsteers/pipelinewise-singer-python/actions

@Samira-El
Copy link
Copy Markdown
Collaborator

Hey Aaron, thanks for the PR.
I ran the mypy command locally on your branch and I'm getting:

mypy --config-file ./mypy.ini singer
singer/utils.py:11: error: Library stubs not installed for "dateutil.parser" (or incompatible with Python 3.8)
singer/utils.py:11: note: Hint: "python3 -m pip install types-python-dateutil"
singer/utils.py:11: error: Library stubs not installed for "dateutil" (or incompatible with Python 3.8)
singer/utils.py:12: error: Library stubs not installed for "pytz" (or incompatible with Python 3.8)
singer/utils.py:27: error: Returning Any from function declared to return "datetime"
singer/utils.py:29: error: Returning Any from function declared to return "datetime"
singer/utils.py:65: error: Returning Any from function declared to return "datetime"
singer/utils.py:67: error: Returning Any from function declared to return "datetime"
singer/messages.py:5: error: Library stubs not installed for "pytz" (or incompatible with Python 3.8)
singer/messages.py:5: note: Hint: "python3 -m pip install types-pytz"
singer/messages.py:5: note: (or run "mypy --install-types" to install all missing stub packages)
singer/messages.py:5: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
singer/messages.py:6: error: Library stubs not installed for "simplejson" (or incompatible with Python 3.8)
singer/messages.py:6: note: Hint: "python3 -m pip install types-simplejson"
singer/messages.py:291: error: Returning Any from function declared to return "str"
Found 10 errors in 2 files (checked 12 source files)

Don't know why CI in your branch is passing ?!🤔

@Samira-El
Copy link
Copy Markdown
Collaborator

Samira-El commented Jun 16, 2021

Also, can you remove the poetry.lock file, we don't use poetry in this project.. yet!

@aaronsteers
Copy link
Copy Markdown
Author

@Samira-El

Also, can you remove the poetry.lock file, we don't use poetry in this project.. yet!

Done! ✅ Sorry that creeped in. 👻

@aaronsteers
Copy link
Copy Markdown
Author

Don't know why CI in your branch is passing ?!🤔

Not sure about this... could be version differences or something else going on...

@edgarrmondragon
Copy link
Copy Markdown
Contributor

edgarrmondragon commented Nov 19, 2021

@aaronsteers I'm seeing the same errors on a fresh clone and install of your branch

(singer-python) > $ mypy --config-file mypy.ini singer
singer/utils.py:11: error: Library stubs not installed for "dateutil.parser" (or incompatible with Python 3.8)
singer/utils.py:11: note: Hint: "python3 -m pip install types-python-dateutil"
singer/utils.py:11: error: Library stubs not installed for "dateutil" (or incompatible with Python 3.8)
singer/utils.py:12: error: Library stubs not installed for "pytz" (or incompatible with Python 3.8)
singer/utils.py:27: error: Returning Any from function declared to return "datetime"
singer/utils.py:29: error: Returning Any from function declared to return "datetime"
singer/utils.py:65: error: Returning Any from function declared to return "datetime"
singer/utils.py:67: error: Returning Any from function declared to return "datetime"
singer/utils.py:226: error: "Exception" has no attribute "response"
singer/messages.py:5: error: Library stubs not installed for "pytz" (or incompatible with Python 3.8)
singer/messages.py:5: note: Hint: "python3 -m pip install types-pytz"
singer/messages.py:5: note: (or run "mypy --install-types" to install all missing stub packages)
singer/messages.py:5: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
singer/messages.py:6: error: Library stubs not installed for "simplejson" (or incompatible with Python 3.8)
singer/messages.py:6: note: Hint: "python3 -m pip install types-simplejson"
singer/messages.py:309: error: Returning Any from function declared to return "str"
Found 11 errors in 2 files (checked 12 source files)

@edgarrmondragon
Copy link
Copy Markdown
Contributor

edgarrmondragon commented Nov 19, 2021

Running

(singer-python) > $ mypy --install-types
Installing missing stub packages:
/Users/edgarramirez/.virtualenvs/singer-python/bin/python3 -m pip install types-python-dateutil types-pytz types-simplejson

fixed all but one error

(singer-python) > $ mypy --config-file mypy.ini singer
singer/utils.py:226: error: "Exception" has no attribute "response"
Found 1 error in 1 file (checked 12 source files)

@aaronsteers
Copy link
Copy Markdown
Author

aaronsteers commented Nov 19, 2021

Running

(singer-python) > $ mypy --install-types
Installing missing stub packages:
/Users/edgarramirez/.virtualenvs/singer-python/bin/python3 -m pip install types-python-dateutil types-pytz types-simplejson

fixed all but one error

(singer-python) > $ mypy --config-file mypy.ini singer
singer/utils.py:226: error: "Exception" has no attribute "response"
Found 1 error in 1 file (checked 12 source files)

Nice! Thanks, @edgarrmondragon! Perhaps when I was running locally via poetry the --install-types was getting executed automatically. I'll see if I can incorporate that command explicitly.

I think the last one is new since I last touched this. Looks like its related to the requests library.

UPDATE: I made a quick attempt to resolve both issues in this commit: 0825e01

Copy link
Copy Markdown
Author

@aaronsteers aaronsteers left a comment

Choose a reason for hiding this comment

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

Per @edgarrmondragon's comment. 👍

Comment thread singer/utils.py Outdated
Comment thread .github/workflows/main.yml
@Samira-El
Copy link
Copy Markdown
Collaborator

Samira-El commented Nov 30, 2021

I had to make this change: d67ad2e for the CI jobs to run on this PR.

@Samira-El
Copy link
Copy Markdown
Collaborator

Looks like Unify is unhappy, we've recently added a linting step to check that only single quotes are used for consistency reasons.

@Samira-El
Copy link
Copy Markdown
Collaborator

Mypy isn't happy too:

$ mypy --config-file ./mypy.ini singer
singer/utils.py:113: error: Returning Any from function declared to return "Union[Dict[Any, Any], List[Any]]"
singer/messages.py:309: error: Incompatible return value type (got "bytes", expected "str")
singer/messages.py:313: error: Argument 1 to "write" of "IO" has incompatible type "str"; expected "bytes"
Found 3 errors in 2 files (checked 12 source files)

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