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

Rename path argument to "dataset" in hdf5_lookup #775

Merged
merged 3 commits into from
Aug 9, 2024

Conversation

jwlodek
Copy link
Member

@jwlodek jwlodek commented Aug 8, 2024

Checklist

  • Add a Changelog entry
  • Add the ticket number which this PR closes to the comment section

Recent ophyd_async change renamed the internal HDF path parameter passed to the stream resource to dataset from path.

Required to merge this in sync with bluesky/bluesky#1790.

See: https://github.com/bluesky/ophyd-async/blob/5ac94e283b170b603df42a7d5fe33fd7f2786524/src/ophyd_async/core/_hdf_dataset.py#L64

@jwlodek jwlodek changed the title Hdf5 rename path to dataset Rename path argument to "dataset" in hdf5_lookup Aug 8, 2024
Copy link
Contributor

@genematx genematx left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks Jakub! I'd still wait for a comment from @danielballan before merging it in case path needs to be changed in any other places that I'm not aware of.

@danielballan
Copy link
Member

I agree the old name was confusing. This needs a SQL migration to update existing DataSource documents.

@danielballan
Copy link
Member

To create a migration script:

$ python -m tiled.catalog revision -m "Change 'path' to 'dataset' in HDF5 assets"
$ git status
On branch main
Your branch is up to date with 'origin/main'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)

       tiled/catalog/migrations/versions/9cfb45fa2c30_change_path_to_dataset_in_hdf5_assets.py

add to the top of this list:

ALL_REVISIONS = [
"ed3a4223a600",
"e756b9381c14",
"2ca16566d692",
"1cd99c02d0c7",
"a66028395cab",
"3db11ff95b6c",
"0b033e7fbe30",
"83889e049ddc",
"6825c778aa3c",
]

In the migration file created above, you need to run an UPDATE command. Example from another migration:

connection.execute(
data_sources.update()
.values(structure_family=structure_family)
.where(data_sources.c.id == data_source_id)
)

@jwlodek jwlodek merged commit 7f7329d into bluesky:main Aug 9, 2024
9 checks passed
@genematx genematx mentioned this pull request Aug 9, 2024
2 tasks
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.

3 participants