Skip to content

Conversation

kc0506
Copy link
Contributor

@kc0506 kc0506 commented Oct 17, 2024

Change Summary

Put type alias logic after from_property.

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

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Oct 17, 2024
Copy link

codspeed-hq bot commented Oct 17, 2024

CodSpeed Performance Report

Merging #10643 will not alter performance

Comparing kc0506:tweak-type-alias (5ebb0ed) with main (7bb0e0a)

Summary

✅ 44 untouched benchmarks

@sydney-runkle
Copy link
Contributor

@Viicos, could you take a look at this one? You've had your head in this space recently...

@sydney-runkle sydney-runkle requested a review from Viicos October 17, 2024 09:27
Copy link
Contributor

github-actions bot commented Oct 17, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _generate_schema.py
Project Total  

This report was generated by python-coverage-comment-action

Comment on lines +1041 to +1042
if isinstance(origin, TypeAliasType):
return self._type_alias_type_schema(obj)
Copy link
Member

Choose a reason for hiding this comment

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

Why was it moved?

Copy link
Contributor Author

@kc0506 kc0506 Oct 17, 2024

Choose a reason for hiding this comment

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

I think without a special reason (e.g. dataclass above), these matching clauses should better be put together to make things clearer. Looks like when origin is of type TypeAliasType, from_property is always None (at least in the current tests), so moving it does not have any actual effect.

@sydney-runkle sydney-runkle added the awaiting author revision awaiting changes from the PR author label Oct 17, 2024
@Viicos Viicos enabled auto-merge (squash) October 17, 2024 10:16
@Viicos Viicos merged commit fcea8ee into pydantic:main Oct 17, 2024
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting author revision awaiting changes from the PR author relnotes-fix Used for bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants