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

FIX: Tensorization script no longer worked for pngs where there is no… #566

Merged
merged 2 commits into from
May 7, 2024

Conversation

daniellepace
Copy link
Collaborator

… ZIP_FOLDER, and could not handle it if the ZIP_FOLDER was not the first python arg

… ZIP_FOLDER, and could not handle it if the ZIP_FOLDER was not the first python arg
find $ZIP_FOLDER -name '*.zip' | xargs -I {} basename {} | cut -d '_' -f 1 \
| awk -v min="$MIN_SAMPLE_ID" -v max="$MAX_SAMPLE_ID" '$1 > min && $1 < max' \
| sort | uniq > /tmp/ml4h/sample_ids_trimmed.txt
elif [ -e "$MANIFEST" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible for people to pass both --zip_folder and --app_csv args? If so, is it ok that this will use the zip_folder args over the manifest if that is the case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! You convinced me to fix this up a bit better =) I've made some changes that more explicity refer to whether a 'tensorize' or 'tensorize_pngs' job is running, so that the right data is pulled from. There are also modes to 'tensorize_ecg_pngs' and 'tensorize_partners', but Sam says no one has done that for a while and we can just have the script fail in those cases.

@daniellepace daniellepace force-pushed the dfp_fix_tensorize_png branch from b15e4f7 to 15827fd Compare May 6, 2024 18:48
Copy link
Collaborator

@abaumann abaumann left a comment

Choose a reason for hiding this comment

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

👍

@daniellepace daniellepace merged commit 3e8b662 into master May 7, 2024
3 checks passed
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