python-itoolkit icon indicating copy to clipboard operation
python-itoolkit copied to clipboard

Add type declarations

Open kadler opened this issue 5 years ago • 17 comments

https://mypy.readthedocs.io/en/latest/cheat_sheet_py3.html https://mypy.readthedocs.io/en/latest/existing_code.html

This would allow us to have better docs using https://pypi.org/project/sphinx-autodoc-typehints/

Ideally, we would use inline type declarations, but they can't be used if the code needs to be Python 2 compatible. For Python 2 compatibility, you must use comments, but this requires Python 3.8 or typed-ast be installed in order for sphinx-autodoc-typehints to be able to read them.

kadler avatar May 21 '20 14:05 kadler

If you or active volunteers (I don't count myself as active) really want this, I won't object, but I won't be a fan.

I am in the camp that has always been and will always remain against in-line type annotations in Python. I think they really destroy the readability of the language, and we don't even get type enforcement or static-typing speed out of the deal. (Unlike many controversial PEPs, PEP 484 doesn't even mention the opposition, but it was vociferous on the python-dev list.)

I would be fully supportive of comment-based (or stub file) type annotations. I think the requirement for Python 3.8 or typed-ast isn't particularly onerous, especially given that it would only be imposed on people who are already willing, able, and invested in setting up and using Sphinx.

jkyeung avatar May 22 '20 00:05 jkyeung

Thanks for the input, @jkyeung. I'm very new to type hints, so I'm not sure the approach that I'd want to take just yet.

The biggest concern I had with Python 3.8 / typed-ast was whether that was supported by readthedocs.io, since that's where the documentation is hosted. I agree that setting up a Python 3.8 environment to build docs is not all that onerous, especially since very few people would be doing so.

kadler avatar May 26 '20 23:05 kadler

Python 2 is no longer an issue, correct? Expired budgie.

jwoehr avatar Apr 07 '23 20:04 jwoehr

Yeah, the current development release (2.0.0-dev) is Python 3.6+

kadler avatar Apr 10 '23 18:04 kadler

The v1 branch still "supports" Python 2, but I don't anticipate many changes there.

kadler avatar Apr 10 '23 18:04 kadler

I support the plan to add type annotation. I don't currently have time to do that work, but I can help review it. Or possibly later this year I may have time to work on it.

jwoehr avatar Apr 10 '23 18:04 jwoehr

@kadler is anyone working on this? @alanseiden and I were chatting on this today and I forked and converted a file. Should I proceed thru the tree?

jwoehr avatar Jun 23 '23 20:06 jwoehr

Trying to run tests but get error:

$ python -m pytest tests
ERROR: usage: __main__.py [options] [file_or_dir] [file_or_dir] [...]
__main__.py: error: unrecognized arguments: --cov=itoolkit
 inifile: /home/jwoehr/work/python-itoolkit/setup.cfg
 rootdir: /home/jwoehr/work/python-itoolkit

Any tips?

jwoehr avatar Jun 23 '23 20:06 jwoehr

IIRC, you need pytest-cov installed. Been a while, but IIRC you can use something like poetry run python -m pytest tests since Poetry manages the venvs for you.

kadler avatar Jun 26 '23 20:06 kadler

@jwoehr feel free to work on it if you like!

kadler avatar Jun 26 '23 20:06 kadler

Thanks, that helps. Doing better now, 87 passed, but 16 fail with an error of the form:

_________________ ERROR at setup of test_idb2call_with_ibm_db __________________
file /home/jwoehr/work/python-itoolkit/tests/test_unit_idb2call.py, line 41
  def test_idb2call_with_ibm_db(mocker, database_callproc):
E       fixture 'mocker' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, cov, database, database_callproc, database_close_exception, database_execute, doctest_namespace, monkeypatch, no_cover, pytestconfig, record_property, record_testsuite_property, record_xml_attribute, recwarn, tmp_path, tmp_path_factory, tmpdir, tmpdir_factory, transport
>       use 'pytest --fixtures [testpath]' for help on them.

/home/jwoehr/work/python-itoolkit/tests/test_unit_idb2call.py:41

mocker is installed

jwoehr avatar Jun 26 '23 20:06 jwoehr

Is it installed in poetry's venv? (eg. poetry run pip env)

kadler avatar Jun 26 '23 20:06 kadler

(test_venv) jwoehr@pop-os:~/work/python-itoolkit$ poetry run pip env
ERROR: unknown command "env"
(test_venv) jwoehr@pop-os:~/work/python-itoolkit$ poetry run pip list --local
Package        Version
-------------- ----------
coverage       7.2.7
exceptiongroup 1.1.1
iniconfig      2.0.0
itoolkit       2.0.0.dev0
mock           5.0.2
packaging      23.1
pip            23.1.2
pluggy         1.2.0
pytest         7.4.0
pytest-cov     4.1.0
PyYAML         6.0
setuptools     59.6.0
tomli          2.0.1
wheel          0.40.0

jwoehr avatar Jun 26 '23 21:06 jwoehr

Sorry, I gave the wrong command but you figured out what I meant.

Hmm, looks at the error message closer it looks like test_idb2call_with_ibm_db is taking a fixture called "mocker". Seems to be provided by pytest-mock, which is not installed in your venv. It's listed in the dev-dependencies in pyproject.toml, though. Seems like they changed things since Poetry 1.2, which uses dependency groups instead now but should still be backward compatible.

Maybe try poetry install --with dev?

kadler avatar Jun 27 '23 15:06 kadler

I got it set up on my new system. Here's what I did:

  • Install poetry https://python-poetry.org/docs/#installation
  • Ran poetry install
  • Ran poetry run python -m pytest

kadler avatar Jun 27 '23 15:06 kadler

There might be a way to do this with pure pip nowadays since some recent PEPs, but I'm not sure on that process.

kadler avatar Jun 27 '23 15:06 kadler

Phew. Got that to work. Side note: one must deactivate the venv one already created for poetry install to work.

--------------------------------------------------------
TOTAL                                  700    247    65%

======================= 103 passed, 2 warnings in 0.33s ========================

jwoehr avatar Jun 27 '23 17:06 jwoehr