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

blueapi cannot generate a JsonSchema for bluesky.protocols.Movable after update to 1.13.1 #843

Open
ZohebShaikh opened this issue Mar 3, 2025 · 6 comments · May be fixed by #844
Open
Assignees
Labels
bug Something isn't working

Comments

@ZohebShaikh
Copy link
Contributor

Describe the Bug

After updating bluesky to version 1.13.1, blueapi fails to generate a JSON schema for bluesky.protocols.Movable, raising the following error:

pydantic.errors.PydanticInvalidForJsonSchema: Cannot generate a JsonSchema for core_schema.IsInstanceSchema (<class 'bluesky.protocols.Movable'>)

This issue affects plans in dodal, specifically in [wrapped.py](https://github.com/DiamondLightSource/dodal/blob/main/src/dodal/plan_stubs/wrapped.py).

Expected Behavior

blueapi should correctly generate a JSON schema for bluesky.protocols.Movable without errors.

Actual Behavior

blueapi fails with PydanticInvalidForJsonSchema, preventing JSON schema generation and causing issues in plans.

Steps To Reproduce

  1. Upgrade bluesky to version 1.13.1.
  2. Use blueapi to generate a JSON schema involving bluesky.protocols.Movable.
  3. Observe the raised PydanticInvalidForJsonSchema error.

Acceptance Criteria

  • blueapi successfully generates a JSON schema for bluesky.protocols.Movable without errors.
  • Plans in dodal function as expected without schema-related failures.
@ZohebShaikh ZohebShaikh added the bug Something isn't working label Mar 3, 2025
@ZohebShaikh ZohebShaikh changed the title blueapi cannot generate a JsonSchema for bluesky.protocols.Movable after update to 0.13.1 blueapi cannot generate a JsonSchema for bluesky.protocols.Movable after update to 1.13.1 Mar 3, 2025
@DiamondJoseph DiamondJoseph self-assigned this Mar 3, 2025
@DiamondJoseph DiamondJoseph linked a pull request Mar 3, 2025 that will close this issue
@stan-dot
Copy link
Contributor

stan-dot commented Mar 3, 2025

how did this get through regression testing?

the acceptance criteria need to have a point to add a test to prevent this from arising in the future.

Alternatively a new issue could be made

@DiamondJoseph
Copy link
Contributor

This didn't get through regression tests. The CI is currently failing when bluesky is not pinned to <=1.13.0.

@DiamondJoseph
Copy link
Contributor

DiamondJoseph commented Mar 3, 2025

What I've got going over at #844 is working to deserialise plans and devices, but looking at how to handle generics in jsonschema compatible ways, I'd like to take a bit more time to get it potentially interoperable with future things, potentially after getting the first approach merged if the need for dodal and bluesky 1.13.1 is urgent and rolling back bluesky to 1.13.0 isn't an option

It's going to take some munging to handle generics and make non-generics look similar to generics:

...
{
  "name": "set_relative",
  "description": "words",
  "schema": {
    "additionalProperties": false,
    "properties": {
      "movable": {
        "title": "Movable",
        "type": "bluesky.protocols.Movable['dodal.plan_stubs.wrapped.T']"
      },
      "value": {
        "title": "Value"
      }
    },
    "required": [
      "movable",
      "value"
    ],
    "title": "set_relative",
    "type": "object"
  }
}

changes to something closer to (still fiddling with the exact expected form)

{"$defs": {    "moveable-axis": {      "$Anchor": "T"    }  },
...
{
  "name": "set_relative",
  "description": "words",
  "schema": {
    "additionalProperties": false,
    "properties": {
      "movable": {
        "title": "Movable",
        "type": "bluesky.protocols.Movable",
        "properties": {"axis": "$moveable-axis"}
      },
      "value": {
        "title": "Value",
        "type": "$moveable-axis"
      }
    },
    "required": [
      "movable",
      "value"
    ],
    "title": "set_relative",
    "type": "object"
  }
}

::

...
{
  "name": "concrete_set",
  "description": "words",
  "schema": {
    "additionalProperties": false,
    "properties": {
      "movable": {
        "title": "Movable",
        "type": "bluesky.protocols.Movable",
        "properties": {"axis": {"type": "float"}}
      },
      "value": {
        "title": "Value",
        "type": "float"
      }
    },
    "required": [
      "movable",
      "value"
    ],
    "title": "set_relative",
    "type": "object"
  }
}

@DominicOram
Copy link
Contributor

need for dodal and bluesky 1.13.1 is urgent and rolling back bluesky to 1.13.0 isn't an option

I have rolled everything back. Happy to stay on 1.13.0 until this is sorted

@stan-dot
Copy link
Contributor

stan-dot commented Mar 4, 2025

@DiamondJoseph isn't on the UI end the type just a string? for device names? for data input

@DiamondJoseph
Copy link
Contributor

It is passed as a string, but the plan parameters are correct in the generated schema, and when we allow querying for devices of type Movable or Movable[float] we need a way of knowing what type of Movable[T] a plan can take (e.g. a SpecScan should only take a Movable[float], so we should be able to identify that from the schema, and find devices that are valid). And if there is a standard way of doing that, then we should use that standard way in the fetched schema and the query params to get the valid devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants