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

Only merge in open_run(..., data='all') if raw is not a subset of proc #236

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

philsmt
Copy link
Contributor

@philsmt philsmt commented Oct 29, 2021

open_run(..., data='all') throws an exception since #208 if raw is completely a subset of proc, because it will try to access the metadata of raw \ proc while merging this into proc. If this set is empty however, the call to DataCollection.run_metadata() fails due to access to files[0].

While this begs a larger question of whether such DataCollection objects such exist in the first place, @takluyver and me agreed to discuss this separately in #235. It is clearly not necessary and thus a bug in open_run do try this in the first place if the DataCollection is empty anyway.

@philsmt philsmt added the bug Something isn't working label Oct 29, 2021
@philsmt
Copy link
Contributor Author

philsmt commented Oct 29, 2021

Now technically this only looks at sources, so if proc adds keys, but does not contain those in raw, it would still exclude the ones on raw and only show the added ones.

For AGIPD, data is actually replaced by calibration. Do we know this is the case for all detectors?

@takluyver
Copy link
Member

I think the offline correction for all detectors uses the same source & key name as the raw data, so effectively replacing data.

EXtra-data currently assumes that each source exists in a single sequence of files, so we can't easily break it down to 'these keys from that file, those keys from another file'.

@philsmt
Copy link
Contributor Author

philsmt commented Oct 29, 2021

Ah, this might've been the reason we sorted only by source in the first place. Thanks!

@takluyver
Copy link
Member

LGTM

@takluyver takluyver added this to the 1.8.1 milestone Oct 29, 2021
@takluyver takluyver merged commit c311fc8 into master Nov 1, 2021
@takluyver takluyver deleted the fix/open-run-with-raw-in-proc branch November 1, 2021 10:03
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 this pull request may close these issues.

2 participants