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

Migrate to higlass-schema v0.2.0 (pydantic v2) #155

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Conversation

manzt
Copy link
Member

@manzt manzt commented Nov 10, 2024

Needed to accommodate the pydantic migration in higlass-schema:

Staging, until that PR is merged.

@manzt manzt changed the title Prepare for higlass-schema pydantic migration Migrate to higlass-schema v0.2.0 (pydantic v2) Nov 12, 2024
@manzt
Copy link
Member Author

manzt commented Nov 12, 2024

Hmm, I'm getting an issue with main and this branch manzt/pydantic-v2 with the notebook.

To reproduce, in the root of this repo:

wget https://gist.githubusercontent.com/manzt/359ac6e2f95d09312e9d365e25096f5e/raw/3f8ebfb9c1cd9fcc1886ac94da32442d8dc9bde6/higlass-example.ipynb
uvx juv add higlass-example.ipynb .
uvx juv run higlass-example.ipynb

main
image

manzt/pydantic-v2
image

I'm not sure why, but the height/width behavior seems to be different for vertical tracks. As in for whatever reason the vertical tracks happily allow height on main, but require width on this branch. The rendering code hasn't changed, so I don't know what the issue is. Also quick inspection of the view configs (view.viewconf().json()) and I don't see differences other than view ids and server ports.

I was able to get consistent behavior between the two with the following diff.

view = hg.view(
++  (hic.track("horizontal-chromosome-labels", height=40), "top"),
--  (hic.track("horizontal-chromosome-labels"), "top"),
++  (hic.track("vertical-chromosome-labels", width=40), "left"),
--  (hic.track("vertical-chromosome-labels"), "left"),
    (hic.track("heatmap"), "center"),
    (genes.track("horizontal-gene-annotations", height=100), "top"),
++  (genes.track("vertical-gene-annotations", width=100), "left"),
--  (genes.track("vertical-gene-annotations"), "left"),
)

view.widget()
image

I sincerely have no idea why this isn't the same or what has changed and I don't have the time to dig in for a bit. I cut a release of higlass-python to pin the version of higlass-schema pre v0.2.0 until it can be resolved. Feel free to push forward if you'd like @nvictus

@nvictus
Copy link
Member

nvictus commented Nov 12, 2024

Thanks for investigating and pinning the schema to pydantic v1 @manzt!

@pkerpedjiev @flekschas, any idea what might be going on? We migrated the schema and model that higlass-python uses to pydantic v2 and jsonschema 2020-12.

There are minor differences in the viewconfs generated but there doesn't seem to be any reason for the track height/width behavior to change.

@manzt
Copy link
Member Author

manzt commented Nov 12, 2024

FWIW, I don't even think the schema.json (from higlass-schema) is used in any way in higlass-python. Inspecting the generated JSON view configs between branches for the same code (view.viewconf().json()), I can't see a difference other than uids.

Maybe worth someone else trying as well, in case I'm doing something wrong.

@nvictus
Copy link
Member

nvictus commented Nov 12, 2024

Trying out myself and saving the viewconfs generated. Seems like there are several explicit null fields produced in the new model.

My guess is the explicit "height": None is overriding some default height on vertical tracks used when the height is not specified at all.

FWIW, I don't even think the schema.json

Right, I think it's pydantic v2 semantics being explicit about filling in missing attributes.

Here's the viewconf from `main`:
{
  "layout": {
    "x": 0,
    "y": 0,
    "w": 12,
    "h": 6
  },
  "tracks": {
    "left": [
      {
        "tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909",
        "server": "http://localhost:60079/tilesets/api/v1/",
        "type": "vertical-chromosome-labels",
        "uid": "4b542bc7-734f-4118-8c67-4ead60d5f57d"
      },
      {
        "tilesetUid": "M9A9klpwTci5Vf4bHZ864g",
        "server": "https://resgen.io/api/v1",
        "type": "vertical-gene-annotations",
        "uid": "6c9498b5-c1e4-4378-840c-7bff5c3ac3e5",
        "height": 100,
        "options": {
          "name": "Gene Annotations (hg38)"
        }
      }
    ],
    "top": [
      {
        "tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909",
        "server": "http://localhost:60079/tilesets/api/v1/",
        "type": "horizontal-chromosome-labels",
        "uid": "22ad835b-a44e-4f86-8f84-20070140d95b"
      },
      {
        "tilesetUid": "M9A9klpwTci5Vf4bHZ864g",
        "server": "https://resgen.io/api/v1",
        "type": "horizontal-gene-annotations",
        "uid": "e1534f2d-6846-42f0-8ba2-8024a4eaa3c5",
        "height": 100,
        "options": {
          "name": "Gene Annotations (hg38)"
        }
      }
    ],
    "center": [
      {
        "tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909",
        "server": "http://localhost:60079/tilesets/api/v1/",
        "type": "heatmap",
        "uid": "6fc53f37-381d-44ce-b940-03921481564d"
      }
    ]
  },
  "uid": "d5f2f7ac-2a69-4d0f-b577-94c9665063b2",
  "zoomLimits": [
    1,
    null
  ]
}
Here's the same viewconf from this branch:
{
  "layout": {
    "x": 0,
    "y": 0,
    "w": 12,
    "h": 6,
    "moved": null,
    "static": null
  },
  "tracks": {
    "left": [
      {
        "tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909",
        "server": "http://localhost:60218/tilesets/api/v1/",
        "type": "vertical-chromosome-labels",
        "uid": "52a07fcc-8af0-416d-91c2-8385ad5b53f8",
        "width": null,
        "height": null,
        "options": null,
        "data": null,
        "chromInfoPath": null,
        "fromViewUid": null,
        "x": null,
        "y": null
      },
      {
        "tilesetUid": "M9A9klpwTci5Vf4bHZ864g",
        "server": "https://resgen.io/api/v1",
        "type": "vertical-gene-annotations",
        "uid": "df206008-387d-4e46-9a6e-43496fae9dc1",
        "width": null,
        "height": 100,
        "options": {
          "name": "Gene Annotations (hg38)"
        },
        "data": null,
        "chromInfoPath": null,
        "fromViewUid": null,
        "x": null,
        "y": null
      }
    ],
    "right": null,
    "top": [
      {
        "tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909",
        "server": "http://localhost:60218/tilesets/api/v1/",
        "type": "horizontal-chromosome-labels",
        "uid": "c88a89e9-9b3c-4c1a-ac91-9f52fad507fe",
        "width": null,
        "height": null,
        "options": null,
        "data": null,
        "chromInfoPath": null,
        "fromViewUid": null,
        "x": null,
        "y": null
      },
      {
        "tilesetUid": "M9A9klpwTci5Vf4bHZ864g",
        "server": "https://resgen.io/api/v1",
        "type": "horizontal-gene-annotations",
        "uid": "e265972e-5f9d-4c59-a4e8-4e634e759c10",
        "width": null,
        "height": 100,
        "options": {
          "name": "Gene Annotations (hg38)"
        },
        "data": null,
        "chromInfoPath": null,
        "fromViewUid": null,
        "x": null,
        "y": null
      }
    ],
    "bottom": null,
    "center": [
      {
        "tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909",
        "server": "http://localhost:60218/tilesets/api/v1/",
        "type": "heatmap",
        "uid": "1d773853-9a67-48f8-bf6c-188f482c7816",
        "width": null,
        "height": null,
        "options": null,
        "data": null,
        "position": null,
        "transforms": null
      }
    ],
    "whole": null,
    "gallery": null
  },
  "uid": "e0dbe3f3-6f2d-4b20-b3c0-1094148af5d3",
  "autocompleteSource": null,
  "chromInfoPath": null,
  "genomePositionSearchBox": null,
  "genomePositionSearchBoxVisible": null,
  "initialXDomain": null,
  "initialYDomain": null,
  "overlays": null,
  "selectionView": null,
  "zoomFixed": null,
  "zoomLimits": [
    1,
    null
  ]
}

@nvictus
Copy link
Member

nvictus commented Nov 12, 2024

view.tracks.left[0].json()

On main:

'{"tilesetUid": "29ff8016cf9e958c468ec6e8f33c2909", "server": "http://localhost:60712/tilesets/api/v1/", "type": "vertical-chromosome-labels", "uid": "e1c1ec93-e2ae-4e80-80a0-0822c68e8ee5"}'

On this branch:

'{"tilesetUid":"29ff8016cf9e958c468ec6e8f33c2909","server":"http://localhost:60774/tilesets/api/v1/","type":"vertical-chromosome-labels","uid":"bcc20827-225f-4fdf-9679-65055cb63376","width":null,"height":null,"options":null,"data":null,"chromInfoPath":null,"fromViewUid":null,"x":null,"y":null}'

@manzt
Copy link
Member Author

manzt commented Nov 12, 2024

hmm, it seems it's related to the explicit nulls, but I believe we are using exclude_none in the dumps.

@nvictus
Copy link
Member

nvictus commented Nov 12, 2024

Okay, it seems like exclude_none=True is not being used in .json(). I thought you implemented that override in higlass-schema @manzt but it seems to have disappeared.

@manzt
Copy link
Member Author

manzt commented Nov 12, 2024

Nice, that was it.

I just migrated the APIs here to prefer the non-deprecated variants, but v0.2.1 will have json/dict with exclude_none=True.

@manzt manzt merged commit 43db278 into main Nov 12, 2024
6 checks passed
@manzt manzt deleted the manzt/pydantic-v2 branch November 12, 2024 14:44
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