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

Store npz file #80

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Store npz file #80

merged 2 commits into from
Jan 5, 2023

Conversation

jwblangley
Copy link
Contributor

@jwblangley jwblangley commented Dec 3, 2021

Summary

As per my own experience and Issue #77, I believe that the ability to save an npz file is a significant missing feature to this otherwise great tool! So I went and implemented it! The user can simply specify the --save-stats flag and the stats of the first path will be generated and saved in the location of the second path.

Changes

  • Add the argument --save-stats to the argument parser
  • Implement save_fid_stats using calcualte_fid_given_paths as a guide and making use of np.savez_compressed
  • In __main__, if the --save-stats flag has been specified, run save_fid_stats and exit early. This design makes it clear that the --save-stats flag is the option and the original workflow is still the primary workflow.
  • Update README.md to document the changes
    • My editor strips trailing whitespace. I believe that this is best practice so I have kept these changes in this PR.

Test Plan

I have manually tested the new workflow and the original and they behave exactly as expected. By logical inspection of the changes I've made, I believe no errors will have been introduced, however, I failed to run the included test suite due to an error that is present on master - prior to my changes. The error I am receiving is:

TypeError: compute_statistics_of_path() got an unexpected keyword argument 'num_workers'

Please advise how to fix this and I can rerun the test suite.

Code formatting

I have done my best to copy the style in which the existing code is written. I could not find a linter that is in use - if there is one, please make me aware of it.

@allglc
Copy link

allglc commented Jan 17, 2022

Thanks for the implementation, it works well for me!

@mseitzer mseitzer merged commit 6889825 into mseitzer:master Jan 5, 2023
@mseitzer
Copy link
Owner

mseitzer commented Jan 5, 2023

Thanks for the PR, looks good to me!

@jwblangley jwblangley deleted the save-stats branch January 8, 2023 22:33
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.

3 participants