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

fix: do not override Document._class_id if it was explicitly set on the document class #1133

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

volfpeter
Copy link

Fixes #1132

@staticxterm
Copy link
Contributor

Hi @volfpeter. Thank you for the issue report/PR.
Please also write tests for the scenarios that you described in the issue report, to have failing tests that test and reproduce this behavior prior to the changes made here in this PR.

@volfpeter
Copy link
Author

Hi, Thanks for the quick response. I just fixed the existing tests. I'll add a new test for this specific case.

@volfpeter
Copy link
Author

@staticxterm I added basically the same set of tests that currently exist for the standard inheritance case. Admittedly, parametrized tests would have been nicer, but I didn't want to start that refactoring, because I have very limited time (and kind of need this fix as soon as possible). Let me know if I should change anything.

@volfpeter
Copy link
Author

volfpeter commented Feb 24, 2025

@staticxterm Do you have an idea why the "3.13, 8.0, 1.10.18" test failed? I ran the test suite locally with the same Python and package versions (Python 3.13, pytest 8.3.4, and both pydantic 1.10.18 and 1.10.21), and all tests passed.

@staticxterm
Copy link
Contributor

Hi @volfpeter, the test failed because yes there is some flakiness. I'll look into it.
I'll rerun the failed test.

@volfpeter
Copy link
Author

Thanks! First I thought this was the case, but then it seemed that locally it's always passing, and the CI job is always failing, and I didn't want to trigger to useless reruns.

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.

Document._class_id is always overwritten
2 participants