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

Improved autocomplete for most ehrql things #2337

Merged
merged 7 commits into from
Jan 7, 2025
Merged

Conversation

rw251
Copy link
Contributor

@rw251 rw251 commented Dec 20, 2024

The type checker/autocomplete now knows:

  • the correct type of all column series (e.g. patients.date_of_birth is a DatePatientSeries)
  • the return type of almost all methods on the series (e.g. clinical_events.numeric_value.sum_for_patient() is a FloatPatientSeries
  • properties on series have the correct type (e.g. patients.date_of_birth.day is a IntPatientSeries)
  • the return type for most operators like +, -, >, / etc.

Copy link

cloudflare-workers-and-pages bot commented Dec 20, 2024

Deploying databuilder-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9548e3d
Status: ✅  Deploy successful!
Preview URL: https://6ebe8e48.databuilder.pages.dev
Branch Preview URL: https://rw-better-autocomplete.databuilder.pages.dev

View logs

Copy link
Contributor

@evansd evansd left a comment

Choose a reason for hiding this comment

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

This is great!

The only part that gave me any pause was the @int_property stuff just because I'm not completely clear on what problem it solves and how it does it. (I'm quite confident there is a problem and that it does solve it, I'd just like to understand it better.)

But it's a small and simple enough bit of code that I'd be happy to merge it now and understand it later.

In terms of testing I'll make a separate ticket to think about options there.

@rw251
Copy link
Contributor Author

rw251 commented Jan 7, 2025

The only part that gave me any pause was the @int_property stuff just because I'm not completely clear on what problem it solves and how it does it. (I'm quite confident there is a problem and that it does solve it, I'd just like to understand it better.)

Having not looked at it since before my break I'm not sure I can remember! It was something to do with how the autocomplete interacted with the @property decorator. I tried a few things, and this seemed to be the simplest. I also considered making it more like a @autocomplete_property[T], but given we currently only have int properties it seemed easier to just do that and not make it more complicated than it needed to be.

@rw251 rw251 merged commit 6002a58 into main Jan 7, 2025
8 checks passed
@rw251 rw251 deleted the rw/better-autocomplete branch January 7, 2025 09:35
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.

2 participants