Skip to content

Migrate API to DataTree from DataSet as core primitive#332

Open
mpiannucci wants to merge 11 commits into
mainfrom
datatree-api
Open

Migrate API to DataTree from DataSet as core primitive#332
mpiannucci wants to merge 11 commits into
mainfrom
datatree-api

Conversation

@mpiannucci
Copy link
Copy Markdown
Contributor

This is intended to be the next major version of xpublish:

  • DataTree is now the core primitive. Rest/SingleDatasetRest accept xr.Dataset | xr.DataTree and normalize internally to Dict[str, DataTree] (bare Datasets become single-node trees). Existing plugins keep working unchanged on the root node.

  • New get_datatree(dataset_id, group) provider hook + matching deps.datatree and group-aware deps.dataset. Plugins can opt into a {group_path:path} URL segment and deps.dataset automatically returns the node at that path; tree-aware plugins use deps.datatree. Legacy get_dataset hook still works (wrapped in a single-node tree) and now emits a DeprecationWarning.

Copy link
Copy Markdown
Member

@abkfenris abkfenris left a comment

Choose a reason for hiding this comment

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

Ok, gonna land comment now even though I haven't poked through the PR as much as I want to, as my browser has already been nuked by the security software and tried to lose what I had already entered. I am liking what I'm seeing so far.

So far the big things that I see is that we might want a migration/upgrade doc with sections specifically focused on server admins (XREDS), and plugin authors.

Comment thread xpublish/plugins/included/dataset_info.py Outdated
Comment thread xpublish/rest.py
Comment thread xpublish/rest.py Outdated
@mpiannucci
Copy link
Copy Markdown
Contributor Author

@abkfenris I would like to try and merge this and #333 tuesday next week. Is that reasonable to you?

@abkfenris
Copy link
Copy Markdown
Member

I'd like got get one of your former XREDS compatriots to provide some feedback as well before landing this.

@srstsavage @jonmjoyce

@mpiannucci
Copy link
Copy Markdown
Contributor Author

Sounds good.

Just to note, that xreds should have no breaking changes associated with this

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