#Copilot issues
real musk
https://github.com/home-assistant/core/pull/165259#discussion_r2910626828
Bug: Python 3.13 and earlier syntax error in except clause. except ValueError, TypeError: is the old Python 2 syntax (which Python 3.14 re-allows as except TypeA, TypeB:). However, even in Python 3.14 where this syntax is valid, except ValueError, TypeError: catches ValueError and binds the exception to the name TypeError, shadowing the built-in TypeError. The intended behavior is to catch both exception types, which requires parentheses: except (ValueError, TypeError):. This means TypeError exceptions will not be caught, and the TypeError built-in will be shadowed in the except block.
## Python Syntax Notes
- Python 3.14 explicitly allows `except TypeA, TypeB:` without parentheses.
muted stone
We can improve the copilot instructions
but memory can also pick it up if we leave a comment on those
real musk
Do I just comment or also ping copilot
muted stone
They are not very clear about that: https://docs.github.com/en/copilot/concepts/agents/copilot-memory#how-memories-are-stored-retained-and-used
Mentioning Copilot will cause it to take action (and thus make a PR); so that doesn't seem the right path
Looking at the memory, there is actually a conflicting memory in there
Let me clean up the first one
past cypress
is it possible to enable access to copilot memory for non-admins of the repo? even read-only would be fine
real musk
I'm also wondering if we can turn copilot off for Syntax things
latent cradle
Looking at the memory, there is actually a conflicting memory in there
Is there a good reason to have memory for these kinds of things? Sounds easy to mess up, especially when not everyone can fix it.
dapper turret
It seems to be confused also by missing from __future__ import annotations
SpotifyData uses SpotifyCoordinator and SpotifyDeviceCoordinator in annotations before those classes are defined, but this module does not use postponed evaluation of annotations. This will raise a NameError at import time when defining SpotifyData. Add from future import annotations at the top of this module (consistent with other spotify files) or quote these annotations (e.g., "SpotifyCoordinator").
I think these are no longer necessary for 3.14
surreal furnace
why not? 3.14 did not complete the transition to the new type annotations?
I mean, there's https://peps.python.org/pep-0749/, but does that mean we can now delete all from __future__ import annotations? If yes, we should do that across the board and add a custom rule disallowing it for consistency.
dapper turret
They are almost always unnecessary now - but they are not 100% no-op
My suggestion is only to avoid in new code and avoid copilot requesting it for now
surreal furnace
Have you tried removing them all?
Some container class libraries are messing with type annotations IIRC, are those affected?
muted stone
Is there a good reason to have memory for these kinds of things? Sounds easy to ...
Memory can be helpful, can also work against one. 🤷♂️ This one can be talked into an advantage and disadvantage
muted stone
is it possible to enable access to copilot memory for non-admins of the repo? ev...
Nope
muted stone
I'm also wondering if we can turn copilot off for Syntax things
Nope, that is not how AI works. You need to instruct it
dapper turret
Have you tried removing them all?
Tests seem to be OK: https://github.com/home-assistant/core/pull/165301
(CI failure is unrelated: lutron)
surreal furnace
OK. Can you improve the PR description a bit? Maybe explain the implementation of PEP 649 in Python 3.14 means it's no longer needed for forward references (that's essentially why we used it AFAIK)? Maybe link to PEP 749?
dapper turret
When I look at homeassistant it is still needed for circular references (in conjunction with if TYPE_CHECKING)
https://github.com/home-assistant/core/pull/165306
For example:
https://github.com/home-assistant/core/blob/6a5e5b663d885433e4e083fa7876b53f21f0d4c3/homeassistant/helpers/entity.py#L71-L72
surreal furnace
I don't understand what you mean
The major benefit offrom __future__ import annotations is that it allows forward references in type annotations
It doesn't change anything wrt to other forward references, that's still limited by some misguided design choices when Python was first created
surreal furnace
OK. We should ban the idiot who added that
It's the same person who implemented the FrozenOrThawed horror, which ALSO is messing with annotations
Damn it
I think both of those hacks were influenced by the stdlib implementation of dataclasses, but I could be mistaken
If they are, maybe the dataclasses implementation in 3.14 is updated
dapper turret
OK. We should ban the idiot who added that
I wonder who that was.... oh right
Anyway - we are drifting from the original "copilot" thread.
I think copilot should stop "recommending" adding from __future__ import annotations
There are a few places where it is still needed - but not many and shouldn't be a blanket recommendation
dapper turret
I found that we cannot blanket remove the header due to https://github.com/python/cpython/pull/145756
However I still feel that copilot should not be recommending it on new code
surreal furnace
I'd rather it always recommend it for consistency. Until the unittest issue is fixed.
dapper turret
Should we close https://github.com/home-assistant/core/pull/165301 then (updating tests)?
surreal furnace
I closed it with a link to the CPython issue
surreal furnace
The bot is recently trying to act more and more like a linter, which it fails spectacularly at. Did you try improving the instructions @real musk ?
Yesterday it claimed the linter would probably force line breaks because a line was probably too long. This is a waste of energy for everyone.
It's also been complaining about mypy probably not liking something, I've seen many BS complaints about type narrowing.