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

Improve some DB expression types #1243

Merged
merged 4 commits into from
Jan 9, 2023
Merged

Improve some DB expression types #1243

merged 4 commits into from
Jan 9, 2023

Conversation

adamchainz
Copy link
Contributor

@adamchainz adamchainz commented Nov 9, 2022

I have made things!

  • Remove several things from GeoAggregate and GeoFuncMixin where the inherited types are correct
  • Add missing kwargs **extra_context to Func.as_sql
  • Remove Agggregate.as_sql since it’s a pass-through to Func.as_sql
  • Reorganize db/models/expressions.pyi to follow the same order as upstream
  • Add missing classes DurationExpression, TemporalSubtraction, Star, and OrderByList

Related issues

n/a

Spotted the problem with Aggregate when adding Mypy to Django-MySQL, which has a custom aggregate class.

@intgr
Copy link
Collaborator

intgr commented Jan 4, 2023

Sorry, lots of conflicts here due to #1253

I promise I will review this if you resolve the conflicts. :)

@adamchainz
Copy link
Contributor Author

Rebased! Tests failed due to an apt issue, fix in #1313.

@sobolevn sobolevn closed this Jan 7, 2023
@sobolevn sobolevn reopened this Jan 7, 2023
Copy link
Collaborator

@intgr intgr left a comment

Choose a reason for hiding this comment

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

Looks good, one import can be cleaned up.

django-stubs/contrib/gis/db/models/aggregates.pyi Outdated Show resolved Hide resolved
self,
expression: Expression | F | Subquery,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reordering the classes here made it very difficult to understand what actually changed here. I had to order them back to get a readable diff.

In the future, if you want to reorder items, please do it as a separate PR without any additional changes.

@intgr intgr merged commit a7a1518 into master Jan 9, 2023
@intgr intgr deleted the expressions branch January 9, 2023 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants