textual

textual copied to clipboard

Same as #3805 but change my branch off main

Hey Dave, thanks for the PR. Would you mind adding a couple of regression tests? 🙌

@rodrigogiraoserrao I can but it won't be pretty because:

The fix requires Python 3.9 (for https://peps.python.org/pep-0585/), but the tests also run on 3.8.

Note: #2867 is a hard crash anyway, so fix is no worse than existing behavior on 3.8.

For tests can either:

1. Defer until 3.8 support is dropped (easiest)
2. Only run test on >= 3.9
3. Expect exception on 3.8, work on >= 3.9

Any preference?

Hey Dave,

Maybe I misunderstood what you're saying but are you suggesting that adding a couple of regression tests and marking them with a @skipif that checks if we're on Python 3.9+ is ugly?

I agree that the new change is no worse than the current behaviour and thus I'm personally a fan of this change but we typically don't add new features without tests!

@rodrigogiraoserrao I was, but I didn't know about skipif, that makes it look quite easy 😁 I'll get some tests in ASAP.

Would I be correct in saying this wouldn't check the generic type, just the class?

i.e. the following would return all DataTable, not just float Datatables?

self.query("*", "DataTable[float]")

I don't think this would correctly restrict the query. i.e. self.query(DataTable[float]) would return all datatables, not just tables of floats. If we can't guarantee that the runtime matches the types, then it's not really viable.

Closing for now, but let me know if I've misunderstood or there is another way to achieve this.