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

Re-organize exports for new validators and serializers #5641

Merged
merged 10 commits into from
May 9, 2023

Conversation

adriangb
Copy link
Member

@adriangb adriangb commented Apr 29, 2023

Selected Reviewer: @samuelcolvin

@cloudflare-pages
Copy link

cloudflare-pages bot commented Apr 29, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 59707b7
Status: ✅  Deploy successful!
Preview URL: https://5314929a.pydantic-docs2.pages.dev
Branch Preview URL: https://validators-serializers-mod.pydantic-docs2.pages.dev

View logs

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

I'm totally in agreement about moving methods to the new modules that make sense, but I really don't think we can remove the methods from __init__.py.

  1. I think knowing that everything in the public API can be imported from pydantic is really helpful
  2. One line imports for pydantic use is much cleaner and simpler
  3. changing this now, especially when we're adding new recommended functions like field_validator will add a lot more churn to the migration

On balance I think this is one of those "correct" vs. "easy" decisions where I tend to lean towards easy and you tend to lean towards correct.

I think on this occasion we should revert that change.

The only exception to the "everything in __init__.py" rule right now (definitely the only important one) is dataclass - on this I think we should:

  1. re-export our version of dataclass
  2. rename the dataclasses module to pydantic_dataclasses to avoid confusion with std-lib dataclasses module.

@adriangb
Copy link
Member Author

If we're going to export something via pydantic/__init__.py I don't think there's much point in also exporting it from another module. Right?

@samuelcolvin
Copy link
Member

I think there is - some people might well want to work like

from pydantic import validators

...
@validators.[tab completion]

More to the point, to "no export from another module" would require all other pydantic modules to be made private, which seems a little ugly.

@adriangb
Copy link
Member Author

adriangb commented May 4, 2023

please review (will solve conflicts)

@pydantic-hooky pydantic-hooky bot assigned hramezani and unassigned adriangb May 4, 2023
@adriangb adriangb closed this May 5, 2023
@adriangb adriangb force-pushed the validators-serializers-mod branch from 18dfb53 to 0abffeb Compare May 5, 2023 13:53
@adriangb adriangb reopened this May 5, 2023
@adriangb adriangb enabled auto-merge (squash) May 5, 2023 13:56
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM.

docs/migration.md Outdated Show resolved Hide resolved
docs/migration.md Outdated Show resolved Hide resolved
docs/usage/exporting_models.md Outdated Show resolved Hide resolved

return dec


@overload
Copy link
Member

Choose a reason for hiding this comment

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

root_validator should surely move into validators.py?

More generally can we remove this module, or move it to _internal.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't move it because:

  1. Backwards compatibility with V1.
  2. We want to deprecate @root_validator anyway

Copy link
Member

Choose a reason for hiding this comment

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

root_validator and validator where in class_validators.py in V1, the logic for supporting imports from the old path is already supported.

I think we move them.

@samuelcolvin
Copy link
Member

I have a fix for the above issues. Please don't push until I've pushed my fix - currently can't due to problems with GH.

@samuelcolvin samuelcolvin assigned adriangb and unassigned hramezani May 9, 2023
@adriangb adriangb merged commit bd463b7 into main May 9, 2023
41 checks passed
@adriangb adriangb deleted the validators-serializers-mod branch May 9, 2023 13:31
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

3 participants