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 information about class in error message of schema generation #8917

Merged
merged 5 commits into from Feb 29, 2024

Conversation

Czaki
Copy link
Contributor

@Czaki Czaki commented Feb 29, 2024

Change Summary

Finally, the last of my dependency reached pydantic 2 compatibility, so I could start migrating my package. I meet an error that requires using a debugger to understand where is a problem as I cannot find which class is responsible for the problem. In this PR I add information about class that have __modify_schema__ and do not have __get_pydantic_json_schema__

Related issue number

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@sydney-runkle sydney-runkle added the relnotes-fix Used for bugfixes. label Feb 29, 2024
Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

@Czaki,

Looks great! Could you please add a test?

Copy link

codspeed-hq bot commented Feb 29, 2024

CodSpeed Performance Report

Merging #8917 will not alter performance

Comparing Czaki:modify_schema_error (c880b1f) with main (bbf0724)

Summary

✅ 10 untouched benchmarks

Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Left a few quick notes. Looks good, assuming you fix the linting errors as well. Thanks a bunch!

tests/test_internal.py Outdated Show resolved Hide resolved
tests/test_internal.py Outdated Show resolved Hide resolved
Co-authored-by: Sydney Runkle <54324534+sydney-runkle@users.noreply.github.com>
@sydney-runkle
Copy link
Member

@Czaki,

You should be able to run make and use pre-commit logic to help you with the linting errors 👍

@Czaki
Copy link
Contributor Author

Czaki commented Feb 29, 2024

I think that I have done it.
I do not know why your pre-commit configuration do not fix formatting and does not print what needs to be fixed.

@sydney-runkle
Copy link
Member

@Czaki,

Looks like you fixed the linting, but your test is failing

@Czaki
Copy link
Contributor Author

Czaki commented Feb 29, 2024

Looks like you fixed the linting, but your test is failing

I see, but your workflow is totally different from this one that I’m familiar with, and it takes me longer time to identify how to test things locally.

Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks!!

@sydney-runkle sydney-runkle enabled auto-merge (squash) February 29, 2024 15:08
@sydney-runkle sydney-runkle merged commit 14e0b7d into pydantic:main Feb 29, 2024
51 checks passed
@Czaki Czaki deleted the modify_schema_error branch February 29, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants