Skip to content

fix issue with nested vector fields and python 3.13 issubclass changes #699

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

Merged
merged 6 commits into from
May 13, 2025

Conversation

sethbuff
Copy link
Collaborator

The main purpose of this PR is to support python 3.13.

There is a change to issubclass where if you feed in a type like list[float], it will raise a type error because it does not validate that the inner types are floats and so it now raises an error instead of possibly returning a false positive.

In troubleshooting this, we also came across this PR: #674 from @huwaizatahir2, making this change (and updating the field to be list[float] instead of list[list[float]] also resolved the issue we were facing.

For backwards compatibility, we still allow list[list[float]] but we wanted to also support what is probably the correct way of defining vector fields which is list[float].

We also kept running into the flaky test from tests/test_hash_model.py::test_pagination_queries so we went ahead and pulled in the fix from this PR: #694

This PR should now make it possible to use redis-om with python 3.13 as well as fix the issues associated with the other 2 PRs tagged above.

Copy link
Collaborator

@abrookins abrookins left a comment

Choose a reason for hiding this comment

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

This look great! 👍 Let's keep an eye on that _is_numeric_type function. If it ends up used elsewhere, we could consider throwing it in utils.py and use a for loop as a performance optimization.

@@ -76,7 +76,7 @@ jobs:
strategy:
matrix:
os: [ ubuntu-latest ]
pyver: [ "3.9", "3.10", "3.11", "3.12", "pypy-3.9", "pypy-3.10" ]
pyver: [ "3.9", "3.10", "3.11", "3.12", "3.13", "pypy-3.9", "pypy-3.10" ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call, we definitely needed to test against 3.13

@sethbuff
Copy link
Collaborator Author

This look great! 👍 Let's keep an eye on that _is_numeric_type function. If it ends up used elsewhere, we could consider throwing it in utils.py and use a for loop as a performance optimization.

That makes more sense, I'll go ahead and throw it in there now to set a precedence for functions like that going forward.

@sethbuff sethbuff merged commit 8ddb6b7 into main May 13, 2025
11 checks passed
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