search

TheAlgorithms / Python

All Algorithms implemented in Python
https://thealgorithms.github.io/Python/
MIT License
191.36k stars 45.21k forks source link

[mypy] Need help to fix all `mypy` errors in the codebase #4052

Closed dhruvmanila closed 2 years ago

dhruvmanila commented 3 years ago

Just one left to fix...

https://github.com/TheAlgorithms/Python/blob/master/mypy.ini#L5

UPDATE: Our GitHub Actions now run mypy --ignore-missing-imports excluding those directories that fail that test.

Currently, we are not running mypy in our regular CI tests as there are a lot of errors in the entire codebase, which needs to be fixed. This won't be a one-person job, so we are asking for help from you. I cannot paste the entire message in here as there are around 600 of them, so here's just a gist of it:

$ mypy --ignore-missing-imports .
strings/word_occurrence.py:17: error: Need type annotation for 'occurrence'
strings/min_cost_string_conversion.py:36: error: No overload variant of "__setitem__" of "list" matches argument types "int", "str"
strings/min_cost_string_conversion.py:36: note: Possible overload variants:
strings/min_cost_string_conversion.py:36: note:     def __setitem__(self, int, int) -> None
strings/min_cost_string_conversion.py:36: note:     def __setitem__(self, slice, Iterable[int]) -> None
strings/min_cost_string_conversion.py:40: error: No overload variant of "__setitem__" of "list" matches argument types "int", "str"
strings/min_cost_string_conversion.py:40: note: Possible overload variants:
strings/min_cost_string_conversion.py:40: note:     def __setitem__(self, int, int) -> None
strings/min_cost_string_conversion.py:40: note:     def __setitem__(self, slice, Iterable[int]) -> None
...
backtracking/n_queens_math.py:109: error: List comprehension has incompatible type List[str]; expected List[int]
backtracking/n_queens_math.py:110: error: Argument 1 to "append" of "list" has incompatible type "List[int]"; expected "List[str]"
backtracking/n_queens_math.py:149: error: Need type annotation for 'boards' (hint: "boards: List[<type>] = ...")
backtracking/minimax.py:15: error: "list" is not subscriptable, use "typing.List" instead
backtracking/knight_tour.py:6: error: "tuple" is not subscriptable, use "typing.Tuple" instead
backtracking/knight_tour.py:6: error: "list" is not subscriptable, use "typing.List" instead
...

Guidelines to follow:

Which errors to fix?

Please follow the below steps to produce all the errors in this library:

git clone --depth 1 https://github.com/TheAlgorithms/Python.git

Then you need to install all the necessary requirements:

cd python/
python -m pip install --upgrade pip
python -m pip install -r requirements.txt
python -m pip install mypy

Then run either of the two commands:

How to fix the errors?

Focus on one directory at a time:

.
├── [x] arithmetic_analysis
├── [x] backtracking
├── [x] bit_manipulation
├── [x] blockchain
├── [x] boolean_algebra
├── [x] cellular_automata
├── [x] ciphers
├── [x] compression
├── [x] computer_vision
├── [x] conversions
├── [ ] data_structures
├── [x] digital_image_processing
├── [x] divide_and_conquer
├── [ ] dynamic_programming
├── [x] electronics
├── [x] file_transfer
├── [x] fractals
├── [x] fuzzy_logic
├── [x] genetic_algorithm
├── [x] geodesy
├── [x] graphics
├── [ ] graphs
├── [x] hashes
├── [x] knapsack
├── [x] linear_algebra
├── [x] machine_learning
├── [ ] maths
├── [ ] matrix
├── [x] networking_flow
├── [x] neural_network
├── [ ] other
├── [ ] project_euler
├── [x] quantum
├── [x] scheduling
├── [x] scripts
├── [ ] searches
├── [x] sorts
├── [ ] strings
└── [x] web_programming

Pre-requisites:

ManuKashyap01 commented 3 years ago

@dhruvmanila I am beginner programmer in python. Is there any help I can do regarding this?

sky3760000 commented 3 years ago

wrking

dhruvmanila commented 3 years ago

Update:

With the latest update for mypy http://mypy-lang.org/, it supports PEP 585 which lets us use list[int] instead of List[int]. We always run on the latest Python version so we will adopt the built in generic types instead of importing it from the typing module.

cclauss commented 3 years ago

I updated the list above to reflect the directories that pass in our build GitHub Action. As contributors fix directories, please add them to the mypy tests in .github/workflows/build.yml so that they get tested and we do not have regressions.

ayushigoyal2840 commented 3 years ago

So, here's the crux of the issue: mypy does not try type-checking every single module you've imported. Instead, it only attempts to type-check modules that have explicitly opted-in to the typing ecosystem.

Modules can opt-in to the typing ecosystem via two key mechanisms:

Add type hints or stubs to their code, and include a file named py.typed within the package they distribute to PyPi (or any other package repository). The presence of this marker makes the package PEP-561-aware. The mypy docs also have more info about PEP-561-aware packages. Alternatively, add stubs to typeshed, the repository of type hints for the standard library and select 3rd party libraries. The aws_xray_sdk package has done neither of these things, so will be ignored by mypy.

cclauss commented 3 years ago

@ayushigoyal2840 The next step is to remove the exclude statement while keeping the tests green. Once that is done, we can look into the missing imports. Once that is done, we can try adding --strict

ikobangs commented 3 years ago

Great work team

cclauss commented 3 years ago

@ikobangs There is still work to do so please consider attacking one of the following directories:

ikobangs commented 3 years ago

Where to start

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

PraneshRajendraKumar commented 3 years ago

I'm a beginner in Python, is there any help I can do?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 years ago

Please reopen this issue once you add more information and updates here. If this is not the case and you need some help, feel free to seek help from our Gitter or ping one of the reviewers. Thank you for your contributions!

ManjunathBharadwaj01 commented 3 years ago

Hello... Can I help with any of the issues here? ..i want to learn the ways ...so i also might need some guidance

murari2401 commented 3 years ago

Hello .. Can I help with any issues here? Like if someone assigns me some work I would be grateful to work on that

cclauss commented 3 years ago

No one assigns work. We just review pull requests so please go ahead.

AminVerve14 commented 3 years ago

Now how to solve it

murari2401 commented 3 years ago

@AminVerve14 Read all the instructions given in the guidelines of this issue, you will get to know

pikulet commented 3 years ago

I have done fixes for the entire proj euler repo

Alireza-Rahimi-3000 commented 3 years ago

its amazing project

marcelozarate commented 3 years ago

@dhruvmanila I believe there are no remaining meaningful fixes to do, as the left ones are adressed by this

Update:

With the latest update for mypy http://mypy-lang.org/, it supports PEP 585 which lets us use list[int] instead of List[int]. We always run on the latest Python version so we will adopt the built in generic types instead of importing it from the typing module.

NumberPiOso commented 3 years ago

@dhruvmanila I believe there are no remaining meaningful fixes to do, as the left ones are adressed by this

Update:

With the latest update for mypy http://mypy-lang.org/, it supports PEP 585 which lets us use list[int] instead of List[int]. We always run on the latest Python version so we will adopt the built in generic types instead of importing it from the typing module.

Indeed when you run command mypy --ignore-missing-imports . it just shows those kind of errors. However, I do see some files that do not have type annotations and are not shown by that.

for example https://github.com/TheAlgorithms/Python/blob/729b4d875a07bd15bc6e5e8a3c79f16fa1e003e6/graphs/a_star.py

Should we be using a different command? or am I missing something?

cclauss commented 3 years ago

Please add from __future__ import annotations and use list, not List because Python 3.10 is already released and we will move to it as soon as opencv-python gets compatible.

ErwinJunge commented 2 years ago

@dhruvmanila I believe there are no remaining meaningful fixes to do, as the left ones are adressed by this

Update:

With the latest update for mypy http://mypy-lang.org/, it supports PEP 585 which lets us use list[int] instead of List[int]. We always run on the latest Python version so we will adopt the built in generic types instead of importing it from the typing module.

Indeed when you run command mypy --ignore-missing-imports . it just shows those kind of errors. However, I do see some files that do not have type annotations and are not shown by that.

for example https://github.com/TheAlgorithms/Python/blob/729b4d875a07bd15bc6e5e8a3c79f16fa1e003e6/graphs/a_star.py

Should we be using a different command? or am I missing something?

The reason this is passing is that we're running mypy in the loosest of settings right now. To increase the actual coverage of type annotations (and get the most benefit), we should use the --strict flag. That's really strict though, so perhaps we can go with --disallow-untyped-defs --check-untyped-defs --disallow-untyped-decorators --strict-equality to begin with and work our way up.

On current master, I get the following results:

Both of these are without all of the ignored directories from mypy.ini. Removing that exclude list leads to

tl;dr: there's still lots of room for improvement here :)

ErwinJunge commented 2 years ago

Whle happily plugging away fixing mypy issues, I inadvertently triggered the bot by opening too many MRs :smile: See https://github.com/TheAlgorithms/Python/pull/5492. I understand the need for the bot, but I was really just following the stated protocol of opening 1 MR per file.

Shall I work by directory instead @dhruvmanila ? It'd save me considerable overhead in creating potentially many MRs, but would lead to probably considerably bigger MRs. I see that @pikulet ran into the same problem fixing the proj euler files and then switched to 1 MR for all the proj euler files.

cclauss commented 2 years ago

I would be willing to live with mypy fixes on a directory-by-directory basis.

I doubt that we need to go all the way to —strict. Having type hints for all function parameters and return types is super helpful but we do not want mypy wound up so tight that beginning developers cannot contribute.

ErwinJunge commented 2 years ago

Going directory by directory now: 3 additional directories done. Back into timeout, because the next PR will make the bot angry ;)

I'm personally aiming for --strict compliance with my contributions (to set a good example), but I agree with @cclaus that we probably don't want to enforce that level due to the chance of scaring off beginning developers. I do think we should (eventually) enforce --disallow-untyped-defs --check-untyped-defs --disallow-untyped-decorators --strict-equality because the first 3 in that list are the useful ones from a documentation/editor integration point of view and the last one often triggers on bugs.

spazm commented 2 years ago

When you get a chance, please open #5648, #5653, and #5656.

I'm over my quota of open pull requests. :)

cclauss commented 2 years ago

5608 Can anyone explain why the exclude in mypy.ini does not work?!?

cclauss commented 2 years ago

What remains??? https://github.com/TheAlgorithms/Python/blob/master/mypy.ini#L5

cclauss commented 2 years ago

Just three left to fix...

https://github.com/TheAlgorithms/Python/blob/master/mypy.ini#L5