feat: Improve BlueapiClient to add plan parameter type hints

[oliwenmandiamond]

Fixes #1505

Improves the help text for a plan to add the type hints and correct default values.

Before change:

>>> bc.plans.grid_analyserscan
grid_analyserscan(analyser, sequence, detectors, args, snake_axes=None, md=None)

>>> bc.plans.count
count(detectors, num=None, delay=None, metadata=None)

With this change

>>> bc.plans.grid_analyserscan
grid_analyserscan(
    analyser: ElectronAnalyserDetector,
    sequence: AbstractBaseSequence,
    detectors: list[Readable],
    args: Any,
    snake_axes: list[Any] | bool | None = None,
    md: dict | None = None
)

>>> bc.plans.count
count(
    detectors: list[Readable],
    num: int = 1,
    delay: float = 0,
    metadata: dict | None = None
)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.72%. Comparing base (6d08154) to head (ac57232).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1469      +/-   ##
==========================================
+ Coverage   95.69%   95.72%   +0.03%     
==========================================
  Files          43       43              
  Lines        3226     3249      +23     
==========================================
+ Hits         3087     3110      +23     
  Misses        139      139              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Alexj9837]

looks good, made two comments and maybe add a test for the added value

[Alexj9837]

would adding something here to deal with null / none

[Alexj9837]

not a huge fan of magic numbers, also will these change? could we set the vars to _REPR_MAX_LENGTH and _REPR_MAX_ARGS_INLINE

[tpoliaw]

I'm not sure about this. I agree that the current repr isn't great but trying to perfectly replicate the function signature is going to lead to edge cases where things don't behave, for instance this gives detectors: list[Readable] for an argument that expects a list of strings (or DeviceRefs). (Possibly fixed by #1121?)

I think this is coming back to a python function not being a perfect fit for an API endpoint. For count, we want group to be optional but we don't want the default to be in the signature (as it makes the schema invalid). I think it would be better if the repr for plans stopped trying to be a signature and became something like Plan("count", (movable, value, group, wait)) and the type/default information was in the description (available via help(bc.plans.count)) as it is already.

If the type stubs PR is merged it could go some of the way towards improving usability. For the 'optional but no default' cases we could have an Unspecified object or similar.

@EmsArnold, you've used the client in a repl a fair bit as well - any thoughts on usability vs correctness?

[tpoliaw]

This still returns a set, no? To be strictly correct, it should probably be dict_keys or collections.abc.Set.

Alternatively make it actually return the whole properties dict

[tpoliaw]

tab = <not a tab> is questionable

Suggested change

	tab = " "
	indent = " "

[tpoliaw]

I think we want to be careful that we're not making an invalid schema. With these changes the schema contains several fields such as

"group": {
    "title": "Group",
    "type": "string",
    "default": null
},

which in turn causes your repr to contain parameters such as metadata: dict = None which is not correctly typed.