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

DN-37738: investigate downstream CI failure due to typing #3

Merged
merged 5 commits into from Jan 27, 2023

Conversation

TallJimbo
Copy link
Member

@TallJimbo TallJimbo commented Jan 27, 2023

Checklist

  • [ ] ran Jenkins skipped because this only touches type annotations
  • added a release note for user-visible changes to doc/changes

@TallJimbo TallJimbo force-pushed the tickets/DM-37738 branch 3 times, most recently from f64af0c to d9333a5 Compare January 27, 2023 17:06
SQLAlchemy now defines ColumnElement as generic, and we don't really
want to make use of that since we usually hold columns in heterogeneous
containers (I imagine this typing is much more useful for the ORM).

In most contexts, not providing a type variable is interpreted as
ColumnElement[Any], but it seems to be causing trouble here in
inheritance involving Generic, as it makes Python want the Generic to
include the typevar for ColumnElement.
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Base: 97.11% // Head: 97.11% // No change to project coverage 👍

Coverage data is based on head (804e38e) compared to base (0583d34).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #3   +/-   ##
=======================================
  Coverage   97.11%   97.11%           
=======================================
  Files          15       15           
  Lines        1246     1246           
  Branches      117      117           
=======================================
  Hits         1210     1210           
  Misses         19       19           
  Partials       17       17           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TallJimbo TallJimbo marked this pull request as ready for review January 27, 2023 17:35
@@ -532,6 +534,7 @@ def _select_to_executable(
`to_payload` for all other relation types.
"""
columns_available: Mapping[ColumnTag, _L] | None = None
executable: sqlalchemy.sql.Select | sqlalchemy.sql.CompoundSelect

Choose a reason for hiding this comment

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

Are we only supporting python 3.10 now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, daf_relation has been 3.10-only from its inception.

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.

None yet

2 participants