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

Add types to variables/functions in project and py.typed file #827

Open
qarmin opened this issue Jun 27, 2023 · 8 comments
Open

Add types to variables/functions in project and py.typed file #827

qarmin opened this issue Jun 27, 2023 · 8 comments
Assignees

Comments

@qarmin
Copy link

qarmin commented Jun 27, 2023

When using Pyright, I found that a lot of errors is visible, because Pyright cannot properly recognize type of variable.

Adding basic types, should help such tools to provide better results for users

simple mypy fpdf(1.4.1) command shows such errors(most should be quite easy to fix)

fpdf/util.py:140: error: Unsupported left operand type for - ("str")  [operator]
fpdf/util.py:145: error: Incompatible return value type (got "tuple[str, str]", expected "tuple[str]")  [return-value]
fpdf/util.py:155: error: Cannot find implementation or library stub for module named "pymemtrace.debug_malloc_stats"  [import]
fpdf/util.py:163: error: Incompatible return value type (got "str", expected "tuple[str]")  [return-value]
fpdf/util.py:166: error: Incompatible return value type (got "str", expected "tuple[str]")  [return-value]
fpdf/line_break.py:375: error: "None" has no attribute "original_character_index"  [attr-defined]
fpdf/line_break.py:452: error: Incompatible types in assignment (expression has type "int", variable has type "None")  [assignment]
fpdf/drawing.py:216: error: Incompatible types in assignment (expression has type "str", target has type "bool")  [assignment]

at the end, mypy should be added to CI to force style

@Lucas-C
Copy link
Member

Lucas-C commented Jun 28, 2023

Hi @qarmin

Thank you for getting in touch.

Do you want to contribute to fpdf2 in order to add type annotations?
You can submit a PR that progressively adds some more typing to fpdf2 methods & functions

@Dave33KK
Copy link

Hello, I´d like to work on this issue.

@andersonhc
Copy link
Collaborator

Hello, I´d like to work on this issue.

I added your name as assignee. Let us know if you have any question.

@Dave33KK
Copy link

Thank you very much @andersonhc . I am working on this issue currently.
Just to be sure I understand this task correctly:
-I would start fixing type hinting for the code where mypy is giving me an error.
-After that I would add mypy to the CI.yml file

Did I miss anything?

Greeting, David

@qarmin
Copy link
Author

qarmin commented Jan 26, 2024

At start I suggest to only add mypy workflow + add mypy.ini that will suppress all errors.
After merging, this will allow Github CI to check every commit that you will produce - In #829 I had the problem that for every commit I made, someone had to approve it to see if the CI would pass, which unfortunately, took too long for me to efficiently fix all the errors, that CI found.

@qarmin
Copy link
Author

qarmin commented Jan 27, 2024

There is even automatic way to add some types into code(but still types needs to be manually verified).
I was not able to run it due error in pytest

  config = pluginmanager.hook.pytest_cmdline_parse(
ImportError while loading conftest '/home/rafal/test/fpdf2/test/conftest.py'.
test/conftest.py:22: in <module>
    from fpdf.util import get_process_rss_as_mib, print_mem_usage
fpdf/__init__.py:22: in <module>
    from .fpdf import (
fpdf/fpdf.py:23: in <module>
    from endesive import signer
venv/lib/python3.11/site-packages/endesive/signer.py:14: in <module>
    from oscrypto import asymmetric
venv/lib/python3.11/site-packages/oscrypto/asymmetric.py:19: in <module>
    from ._asymmetric import _unwrap_private_key_info
venv/lib/python3.11/site-packages/oscrypto/_asymmetric.py:27: in <module>
    from .kdf import pbkdf1, pbkdf2, pkcs12_kdf
venv/lib/python3.11/site-packages/oscrypto/kdf.py:9: in <module>
    from .util import rand_bytes
venv/lib/python3.11/site-packages/oscrypto/util.py:14: in <module>
    from ._openssl.util import rand_bytes
venv/lib/python3.11/site-packages/oscrypto/_openssl/util.py:6: in <module>
    from ._libcrypto import libcrypto, libcrypto_version_info, handle_openssl_error
venv/lib/python3.11/site-packages/oscrypto/_openssl/_libcrypto.py:9: in <module>
    from ._libcrypto_cffi import (
venv/lib/python3.11/site-packages/oscrypto/_openssl/_libcrypto_cffi.py:44: in <module>
    raise LibraryNotFoundError('Error detecting the version of libcrypto')
E   oscrypto.errors.LibraryNotFoundError: Error detecting the version of libcrypto

But probably something like this should generate some types

git clone https://github.com/py-pdf/fpdf2.git
cd fpdf2
sudo apt-get update && sudo apt-get install ghostscript libjpeg-dev swig
python3 -m venv venv
source venv/bin/activate
python -m pip install --upgrade pip setuptools wheel
pip install -r test/requirements.txt
pip install -r contributors/requirements.txt
pip install -r docs/requirements.txt
pip install monkeytype pytest mypy
monkeytype run `which pytest` 
while IFS= read -r line; do monkeytype apply "$line"; done < <(monkeytype list-modules)

@Lucas-C
Copy link
Member

Lucas-C commented Apr 30, 2024

E oscrypto.errors.LibraryNotFoundError: Error detecting the version of libcrypto

You may want to check the version of OpenSSL that you have installed: https://community.snowflake.com/s/article/Python-Connector-fails-to-connect-with-LibraryNotFoundError-Error-detecting-the-version-of-libcrypto

@YuvalBubnovsky
Copy link

E oscrypto.errors.LibraryNotFoundError: Error detecting the version of libcrypto

You may want to check the version of OpenSSL that you have installed: https://community.snowflake.com/s/article/Python-Connector-fails-to-connect-with-LibraryNotFoundError-Error-detecting-the-version-of-libcrypto

This link was great help as I was facing the same issue.

My resolution from this was downgrading oscrypto dependency in my venv to get the tests to run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants