Updates for typeguard

[caneff]

This brings typeguard up to a more current version. Requires accessing protected members but so did the existing code so shrug.

[nanne-aben]

Hi @caneff! Thanks for your contribution, this is very useful! I've enabled you to run the ci/cd now (next time it should also run without me needing to give you permission). There's a type error somewhere. Do you want to have a look at it? Or shall I have a go?

[caneff]

I can do it. How can I run the pre commit locally before pushing? I am still pretty new to working outside of the Google ecosystem.

I know with other packages there was a way to set up pre-commit hooks. What is the steps here?

[nanne-aben]

Ah, yeah, you're right, I haven't set up a pre-commit hook here yet... Let's see if I can do that real quick (you'll probably have solved the errors before I get that going though :p ).

[nanne-aben]

Here you go! Lemme know if that works.
https://strictly-typed-pandas.readthedocs.io/en/latest/contributing.html

[caneff]

Here you go! Lemme know if that works. https://strictly-typed-pandas.readthedocs.io/en/latest/contributing.html

I notice even on main mypy doesn't pass for me:

pre-commit run --all-files
flake8...................................................................Passed
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

docs/source/conf.py:38: error: Need type annotation for "exclude_patterns" (hint: "exclude_patterns: List[<type>] = ...")  [var-annotated]
Found 1 error in 1 file (checked 4 source files)
setup.py:1: error: Skipping analyzing "setuptools": module is installed, but missing library stubs or py.typed marker  [import]
setup.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 4 source files)
tests/test_type_validation.py:36: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/test_type_validation.py:39: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
tests/test_type_validation.py:128: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
Success: no issues found in 4 source files
Success: no issues found in 1 source file

pytest...................................................................Passed

Can you fix this so I can have a clean starting point? It is looking like if we wanted typeguard 4 it is going to need to be a bigger refactor though (which I have started).

[caneff]

@nanne-aben OK, so here is more what I think needs to be done with typeguard 4, going off https://typeguard.readthedocs.io/en/stable/extending.html

But, for every test I try to run I get RecursionError: maximum recursion depth exceeded while calling a Python object. Was wondering if you had thoughts or if this is why you were accessing protected things.

Looks like this mechanism for adding checkers was only added with typeguard 3.

Not asking to completely debug, just if you had thoughts off the top of your head how to proceed in the debugging.

[nanne-aben]

Regarding mypy: in the ci/cd, I specifically run mypy ./strictly_typed_pandas/ ./test/. But I agree that's a bit tedious to type while you're working. If you rebase on main, you'll get an update such that it works with just mypy .. Be sure to reinstall the requirements in your environment (I added stubs for setuptools).

Regarding the other error, lemme have a look.

[nanne-aben]

I'm not sure yet, tbh. Here's what I found.

1. We were running pytest with the typeguard flag, i.e. pytest --typeguard-packages=strictly_typed_pandas,tests.
2. When I remove that flag and instead annotate the two functions used to test typeguard using @typechecked, it behaves as it should.
3. I had to make a few small changes to get it to work properly, specifically typeguard.qualified_name() seems to have been removed. This now means that the error message is formatted a little bit less nicely, but well...
4. They must have changed something in the way the --typeguard-packages flag works. Maybe it doesn't like that DataSet is defined within the scope that we're passing to --typeguard-packages? Maybe that somehow results in a recursion loop? Not sure yet.
5. I think it would be good to create a small test project that uses strictly_typed_pandas (rather than implements it) and run the tests there with --typeguard-packages=test_project. Just to check that that still works.

Btw, I just noticed there's now an error in the notebook stage of the cicd too... I have to check out now though. Will get back to it later!

[nanne-aben]

The notebook issue appears to be a known bug in typeguard...
agronholm/typeguard#364

I'll replace that block of code by a markdown block so it works again. Pity though that they've changed the interface in a way that it breaks usage in notebooks.

[caneff]

OK I've added the last failing mypy thing, and fixed the pre-commit hooks too.

Note that the error message doesn't have argname any more which is unfortunate, but I think it is the same issue as your typeguard.qualified_name() issue. If you know any way to get the name back in there I'm all ears.

Please take another look?

[caneff]

@nanne-aben I'm inclined to drop this. I've encountered other discussions about typeguard > 2 having gone off the rails, being incompatible with jaxtype because it does some fragile reparsing of source code and it being not really maintained any more.

Thoughts?

[nanne-aben]

To answer to an earlier message

I think it would be good to create a small test project that uses strictly_typed_pandas (rather than implements it) and run the tests there with --typeguard-packages=test_project. Just to check that that still works.

I can confirm that that works.

@nanne-aben I'm inclined to drop this. I've encountered other discussions about typeguard > 2 having gone off the rails, being incompatible with agronholm/typeguard#353 because it does patrick-kidger/jaxtyping#124 (comment) and it being not really agronholm/typeguard#198 any more.

Hmmm, I agree that this doesn't look great. Some thoughts:

1. Let's indeed abandon this PR. Can you keep the branch though for future reference? If we ever want to switch after all?
2. Let's make a new branch where we bump to the latest typeguard 2 version.
3. Typeguard has very few dependencies (and typeguard 2 doesn't appear to have any at all), so it's unlikely that using an old version will lead to conflicts with other packages.
4. The only problem would be if people want to use both the old and new version of typeguard, but well...
5. In the future, we should probably think about moving to a different package than typeguard (Beartype is mentioned in one of the threads you posted, but I haven't really looked at it yet). For backward compatibity we could still support typeguard 2 at that point, though as an optional requirement.

[caneff]

To answer to an earlier message

I think it would be good to create a small test project that uses strictly_typed_pandas (rather than implements it) and run the tests there with --typeguard-packages=test_project. Just to check that that still works.

I can confirm that that works.

@nanne-aben I'm inclined to drop this. I've encountered other discussions about typeguard > 2 having gone off the rails, being incompatible with agronholm/typeguard#353 because it does patrick-kidger/jaxtyping#124 (comment) and it being not really agronholm/typeguard#198 any more.

Hmmm, I agree that this doesn't look great. Some thoughts:

1. Let's indeed abandon this PR. Can you keep the branch though for future reference? If we ever want to switch after all?

Will do

2. Let's make a new branch where we bump to the latest typeguard 2 version.

Isn't it already? 2.13.3 is what is in requirements.txt and seems to be the latest 2.

[nanne-aben]

Isn't it already? 2.13.3 is what is in requirements.txt and seems to be the latest 2.

Oh, I thought you'd said it wasn't the latest yet. I hadn't checked it myself. But if it's the latest already, then we can just leave it as it is!

[willhardy]

If I'm not mistaken, pandera requries typeguard > 4, so it is no longer possible to have both strictly_typed_pandas and pandera in the same project.

One option would be to vendor typeguard v2, you aren't using any upstream changes anyway. It would be a quick, safe change that frees projects from the above problem.

[nanne-aben]

Hmmm, yeah, that's a good idea! I've checked the license and tpeguard 2.13.2 (latest v2) is also under an MIT license, so that's fine.

So off the top of my head, we need to:

1. Include the v2.13.2 source code in our repo
2. Update the documentation such that the different ways of enabling typeguard (decorator, import hook and pytest plugin) are enabled on the vendored version of the code
3. Writing a little text instructing users how to transfer to this new setup
4. Remove the typeguard dependency

I'm currently on vacation, so I don't have a lot of time to contribute to this right now. Would you be interested in contributing a PR here?

[willhardy]

I had a look into it and most problems looked to be surmountable, but I think typeguard's "importhook" does something that can only be done by once by one library, so it would get a little messy.

I wanted a clean, works-in-every-environment, no-changes-necessary solution, but couldn't find one; I think getting this to work will need some compromises to be made. I'll make a fork and push my branch over there with some comments about the possible choices.

[nanne-aben]

That's great, thanks!

[willhardy]

For future reference, this is the PR: #142