Skip to content

Comments

fix(deps): update dependency pytango to v10.1.3#1195

Open
renovate[bot] wants to merge 7 commits intomainfrom
renovate/pytango-10.x
Open

fix(deps): update dependency pytango to v10.1.3#1195
renovate[bot] wants to merge 7 commits intomainfrom
renovate/pytango-10.x

Conversation

@renovate
Copy link
Contributor

@renovate renovate bot commented Feb 5, 2026

This PR contains the following updates:

Package Change Age Confidence
pytango (changelog) ==10.0.3==10.1.3 age confidence

Release Notes

tango-controls/pytango (pytango)

v10.1.3

Compare Source

Documentation

More details in the full changelog 10.1.2...10.1.3

v10.1.2

Compare Source

Fixed
Documentation

More details in the full changelog 10.1.1...10.1.2

v10.1.1

Compare Source

Fixed
Documentation
DevOps and code maintenance changes

More details in the full changelog 10.1.0...10.1.1

v10.1.0

Compare Source

Added
Removed
Changed
Fixed
Documentation
DevOps and code maintenance changes

v10.0.4: v10.0.04

Compare Source

Documentation

More details in the full changelog 10.0.3...10.0.4


Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR was generated by Mend Renovate. View the repository job log.

@coretl
Copy link
Collaborator

coretl commented Feb 5, 2026

@burkeds the tests seem to be failing on pytango 10.1.2, do you know why?

@burkeds
Copy link
Collaborator

burkeds commented Feb 5, 2026

I don't but I've been meaning to do a review of event subscriptions once I had Command sorted out.

I saw some inconsistent failures with Tango devices that have multiple Hinted event backed signals. There might be a race condition I overlooked or that might have been introduced in the latest pytango.

@renovate renovate bot force-pushed the renovate/pytango-10.x branch 2 times, most recently from b3d86d9 to d2baffd Compare February 9, 2026 06:09
@renovate renovate bot force-pushed the renovate/pytango-10.x branch from d2baffd to 278f4f5 Compare February 11, 2026 09:54
@renovate renovate bot changed the title fix(deps): update dependency pytango to v10.1.2 fix(deps): update dependency pytango to v10.1.3 Feb 11, 2026
@burkeds
Copy link
Collaborator

burkeds commented Feb 13, 2026

I'll spend some time looking into this today.

…gument was deprecated from subscribe_event. The exception was then lost to the event loop. I added a try/except block and logger message to hopefully make this easier to notice in the future. I also make the class attributes in AttributeProxy into instance attributes which they probably should have been in the first place. I will likely refactor CommandProxy when TangoCommands are added
@burkeds
Copy link
Collaborator

burkeds commented Feb 13, 2026

@burkeds the tests seem to be failing on pytango 10.1.2, do you know why?

I pushed a fix. The stateless arg of DeviceProxy.subscribe_event was deprecated. This raised an exception lost to the event loop. Commenting it out fixes it.

@renovate
Copy link
Contributor Author

renovate bot commented Feb 13, 2026

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

@burkeds
Copy link
Collaborator

burkeds commented Feb 13, 2026

This seems to have completely broken type-checking somehow...

@coretl
Copy link
Collaborator

coretl commented Feb 13, 2026

Looks like DeviceProxy has sprouted some type hints, which are unfortunately all for the sync version. Maybe this is worth bringing up with the Tango devs?

@burkeds
Copy link
Collaborator

burkeds commented Feb 13, 2026

Looks like DeviceProxy has sprouted some type hints, which are unfortunately all for the sync version. Maybe this is worth bringing up with the Tango devs?

Right, I vaguely remember Yury mentioning that. I'll sort this out next week.

@coretl coretl mentioned this pull request Feb 18, 2026
@burkeds
Copy link
Collaborator

burkeds commented Feb 23, 2026

Pytango does something weird.

tango.asyncio.DeviceProxy is an awaitable partial function that returns an object typed like tango.DeviceProxy except its methods are awaitable. This is not reflected in its typehints. This is an async proxy design pattern which effectively monkey patches an async interface onto DeviceProxy at runtime.

I created an issue on the pytango GitLab https://gitlab.com/tango-controls/pytango/-/issues/750

Since this pattern is runtime generated I expect the only solution will be to type: ignore or cast anything that relies directly on the DeviceProxy.

…s an async interface over DeviceProxy which makes like difficult for static type checkers. Unless typing in pytango is widened to accomodate this use case in future versions, we will need to type: ignore just about anything that directly uses the tango DeviceProxy.
…of DevState. get_python_type now raises an error if you try to find the type of an IMAGE of strings or enums. These are not valid tango types.
… _subscribe_to_event from a synchronous function so I removed the RuntimeError exception. Added a test to ensure the logger.debug message appears if the subscription fails.
@burkeds burkeds requested a review from coretl February 23, 2026 12:32
@coretl
Copy link
Collaborator

coretl commented Feb 23, 2026

@claude given the GitLab issue https://gitlab.com/tango-controls/pytango/-/issues/750 is there anything better we can do here?

@claude
Copy link

claude bot commented Feb 23, 2026

Claude finished @coretl's task in 2m 54s —— View job


Analysis

  • Read changed files to understand current approach
  • Assess alternatives to scattered type: ignore comments

On the typing approach

The current approach adds ~15 # type: ignore comments scattered across _tango_transport.py and _signal.py. While workable, there is a cleaner alternative that doesn't require waiting for an upstream fix.

Better approach: define a Protocol and cast once at proxy creation

You could define an AsyncDeviceProxyProtocol that describes the async interface actually used in this codebase, then cast the result of await AsyncDeviceProxy(...) to it once. All downstream call sites in TangoProxy, AttributeProxy, CommandProxy would then get real type checking with no per-site # type: ignore.

from typing import Awaitable, Protocol

class AsyncDeviceProxyProtocol(Protocol):
    def get_attribute_list(self) -> list[str]: ...
    def get_command_list(self) -> list[str]: ...
    def name(self) -> str: ...
    def subscribe_event(
        self, attr_name: str, event_type: EventType, callback: Any, **kwargs: Any
    ) -> Awaitable[int]: ...
    def unsubscribe_event(self, eid: int, **kwargs: Any) -> Any: ...  # sync and async
    def read_attribute(self, attr_name: str) -> Awaitable[DeviceAttribute]: ...
    def write_attribute(self, attr_name: str, value: Any) -> Awaitable[None]: ...
    def get_attribute_config(self, attr_name: str) -> Awaitable[AttributeInfoEx]: ...
    def get_command_config(self, cmd_name: str) -> Awaitable[CommandInfo]: ...

Then proxy creation becomes:

proxy = cast(AsyncDeviceProxyProtocol, await AsyncDeviceProxy(trl))  # type: ignore[misc]

And TangoProxy._proxy would be typed as AsyncDeviceProxyProtocol rather than DeviceProxy. This would eliminate ~12 of the ~15 new # type: ignore comments and give the type checker actual visibility into which methods are expected.

One complication: unsubscribe_event is called both ways:

  • await self._proxy.unsubscribe_event(eid) (async)
  • self._proxy.unsubscribe_event(self._eid, green_mode=False) (sync override)

Typing this as -> Any handles it pragmatically, though it's not elegant. An @overload in the Protocol based on the green_mode kwarg would be more precise but is probably overkill here.

Is it worth it now? The Protocol approach trades ~15 small scattered suppressions for 1 well-documented suppression and a Protocol that doubles as documentation of the interface contract. When pytango eventually fixes the issue (or if it's fixed via a custom stub in the project), removing the cast is a one-line change.

That said, the current PR is already pragmatic and correct — this is a refinement question. If you'd rather not add complexity now, the # type: ignore approach is reasonable.


Other notes on the diff:

  • The _rel_change default changed from class-level 0.1 to instance-level None. This is correct (avoids shared mutable state), but means polling without calling set_polling no longer applies a 10% relative change threshold — it falls through to the periodic flag == 0 trigger instead. Probably intended given set_polling always sets the default.
  • stateless=True removal is necessary — it was deprecated in pytango 10.1.2 and was the root cause of the failures noted in the comments.
  • The raise TypeError for IMAGE-of-str/enum instead of returning Sequence[Sequence[str]] is a reasonable hardening — those types were never actually usable with the rest of the stack.

Copy link
Collaborator

@coretl coretl left a comment

Choose a reason for hiding this comment

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

@burkeds I leave it up to you whether you implement the protocol idea, or just merge this as is...

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