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

DM-34340: Adapt to daf_butler API changes. #118

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Conversation

TallJimbo
Copy link
Member

@TallJimbo TallJimbo commented Nov 16, 2023

{Summary of changes. Prefix PR title with ticket handle.}


  • Passes Jenkins CI.
  • Documentation is up-to-date.

Preview the docs at: https://pipelines.lsst.io/v/DM-FIXME

@@ -125,7 +125,7 @@ def check_stdout(kind, n, last_line, contained, **kwargs):
self.assertIn(contained, result)

# default call with no kwargs
contained = "{instrument: 'TestCam', detector: 25, visit: 54321, ...}"
contained = "{instrument: 'TestCam', detector: 25, visit: 54321, band: '447', physical_filter: '346'}"
Copy link
Member

Choose a reason for hiding this comment

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

I think printing just the "basic" dimensions was actually a useful feature (though I don't know @parejkoj's thoughts on this). Is there no way to do that with the new system?

Copy link
Member Author

Choose a reason for hiding this comment

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

Happy to switch it back, though the "..." will disappear. Since it was hard to print the non-required values if and only if they were present before (but that's easy now), I've been converting stringifications that had the "..." (indicating that available values were not being printed) to print all available values. But it doesn't have to be that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong opinion on this, though having shorter dataIds can keep output from being too cluttered with keys that are irrelevant for a given use (but irrelevancy is often in the eye of the beholder).

Copy link
Member Author

Choose a reason for hiding this comment

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

I have restored this stringification tested here to the concise version (now spelled str(data_id.required)), so the only change to the test is now dropping the , ... from the expected string.

@TallJimbo TallJimbo merged commit d85386c into main Nov 22, 2023
2 checks passed
@TallJimbo TallJimbo deleted the tickets/DM-34340 branch November 22, 2023 04:15
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.

None yet

3 participants