Skip to content

Conversation

dmontagu
Copy link
Contributor

Closes #6548

@@ -702,8 +702,6 @@ def _generate_dc_field_schema(
)

def _common_field_schema(self, name: str, field_info: FieldInfo, decorators: DecoratorInfos) -> _CommonField:
assert field_info.annotation is not None, 'field_info.annotation should not be None when generating a schema'
Copy link
Contributor Author

@dmontagu dmontagu Jul 11, 2023

Choose a reason for hiding this comment

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

I thought this assertion was safe and that we couldn't hit this branch, but apparently we need to handle this case.

I think ideally the defaults for FieldInfo would be PydanticUndefined rather than None, in which case it would make sense to retain the check. But that would be a breaking change at this point.

Copy link
Member

Choose a reason for hiding this comment

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

Would that be an unreasonable breaking change? It's to fix a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, since people do inspect the FieldInfos. And it's not necessary here, that check was just to catch it if there was some other more fundamental bug which presumably doesn't exist (currently). I think it's better to just drop the check and a small amount of hypothetical safety instead of a change that will almost certainly cause issues for at least some people.

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 90b2819
Status: ✅  Deploy successful!
Preview URL: https://e40fea2a.pydantic-docs2.pages.dev
Branch Preview URL: https://none-generic-parameter.pydantic-docs2.pages.dev

View logs

@dmontagu dmontagu merged commit 1e50b0d into main Jul 12, 2023
@dmontagu dmontagu deleted the none-generic-parameter branch July 12, 2023 13:33
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.

Using None as generic type errors in V2.
2 participants