Skip to content

split validators to help avoid circular dependencies#2463

Closed
dmarek-flex wants to merge 1 commit into
developfrom
dmarek/split-validators
Closed

split validators to help avoid circular dependencies#2463
dmarek-flex wants to merge 1 commit into
developfrom
dmarek/split-validators

Conversation

@dmarek-flex
Copy link
Copy Markdown
Contributor

Can we do something like in this PR to help avoid circular dependencies? In my upcoming feature branch ModeSpec is importing my own module which itself importing validators. I already resolved many other circular dependencies by splitting my own modules, but this seems like a fairly reasonable split to me.

@dmarek-flex dmarek-flex requested a review from momchil-flex May 14, 2025 16:31
@dmarek-flex
Copy link
Copy Markdown
Contributor Author

@yaugenst-flex For your Pydantic knowledge

@yaugenst-flex
Copy link
Copy Markdown
Collaborator

One way to get around this is to just import the module within the validator. It's a relatively common pattern in Python, but it does hide the module dependencies somewhat. I was also getting pretty confused with all the different places where we put validators in #2433 so eventually this all should be cleaned up.

In that light, I think this change is ok for now if you need it. We have a lot of circular dependencies that we generally manage to get around by just throwing everything into one big file...

Copy link
Copy Markdown
Collaborator

@momchil-flex momchil-flex left a comment

Choose a reason for hiding this comment

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

Ah. Well this is fine by me and is kind of how we've been doing things. :D

@dmarek-flex
Copy link
Copy Markdown
Contributor Author

great ill just make these changes directly in my other PR

@dmarek-flex dmarek-flex deleted the dmarek/split-validators branch May 16, 2025 14:15
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