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

Guard against ESM1.5 file naming collisions #133

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

blimlim
Copy link
Collaborator

@blimlim blimlim commented Oct 8, 2024

This PR closes #124

It adds a new step to the convert_esm1p5_output_dir function, where pairs of (input_path, output_path) tuples are removed prior to the conversion step if their output_paths are not unique, and warns the user when this occurs.

aidanheerdegen
aidanheerdegen previously approved these changes Oct 11, 2024
Copy link
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

One tiny typo, but otherwise looks good to me.

This option seems like a decent balancing act, and it doesn't remove the original files if they aren't converted, right?

umpost/conversion_driver_esm1p5.py Outdated Show resolved Hide resolved
Co-authored-by: Aidan Heerdegen <[email protected]>
@blimlim
Copy link
Collaborator Author

blimlim commented Oct 13, 2024

This option seems like a decent balancing act, and it doesn't remove the original files if they aren't converted, right?

Yep! Only files that get pushed through the conversion are candidates for deletion, and so any files with clashing outputs won't get deleted.

@blimlim
Copy link
Collaborator Author

blimlim commented Oct 13, 2024

I've just pinged for a re-review – the only change is the typo fix.

Copy link
Member

@aidanheerdegen aidanheerdegen 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 merge to me.

@blimlim blimlim merged commit 0ca826c into main Oct 14, 2024
4 checks passed
@blimlim blimlim deleted the 124/esm-filename-collisions branch October 14, 2024 00:55
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.

ESM1.5 driver: guard against file name collisions
2 participants