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

Remove --reset option from provenance.py (force to always write history from scratch) #311

Merged
merged 8 commits into from
Apr 11, 2023

Conversation

alexdunnjpl
Copy link
Contributor

🗒️ Summary

Removes option to write iterative provenance updates, as this does not improve performance and may result in invalid provenance history in some edge cases.

Also involves extensive renaming and comments in an attempt to improve documentation of provenance script and speed up future changes.

⚙️ Test Data and/or Report

@jimmie do we have any tests that incorporate provenance? I've tested manually against a minimal dataset.

♻️ Related Issues

fixes #310

If one/some reviewers could sanity-check the comments/renaming, I'd appreciate it - I'm pretty confident that I've captured what's actually going on, but extra eyes never hurt. Most of the individual commits should be simple-ish to read, taken individually.

log.info(' aggregate lidvids into lid buckets')
aggregates = {lid: [] for lid in lids}
for lidvid in provenance: aggregates[lidvid.split('::')[0]].append(lidvid)
log.info(' ...binning LIDVIDs by LID...')
Copy link
Member

Choose a reason for hiding this comment

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

Me being pedantic: use an actual ellipsis character instead of three periods ... 😅

Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nutjob4life nutjob4life merged commit f558ddc into main Apr 11, 2023
@nutjob4life nutjob4life deleted the edunn-provenance-wip branch April 11, 2023 13:56
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.

Remove provenance --reset option
2 participants