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

Small fixes #12

Merged
merged 4 commits into from
Jul 25, 2024
Merged

Small fixes #12

merged 4 commits into from
Jul 25, 2024

Conversation

afmagee42
Copy link
Collaborator

Cleaning up a few things I found when trying to run the evaluation demo on a fresh repo copy.

The only actual bugs were:

  1. We needed to be creating the cache directory structure earlier; with ..., save_path.open("wb") as out_file otherwise errors on a fresh repository.
  2. Use of the cache was being set to different default values in different places. I changed things to make the default to use it.

The other changes are a bit more stylistic and/or pedantic:

  1. I replaced the contents of data/README.md with "see the help function" because we don't want to have multiple sources of truth (one is likely to end up out of sync otherwise).
  2. I made the path to included-divisions.txt an argument in load_metadata with a default, rather than a strictly hard-coded value. In so doing, I decided the default place to call this from was the top level of the repo, rather than /data.
  3. I changed the evaluation demo workflow slightly.

@thanasibakis
Copy link
Collaborator

Great catches, thanks!

@afmagee42 afmagee42 merged commit 429ac63 into first-metric Jul 25, 2024
1 check passed
@afmagee42 afmagee42 deleted the afm-fresh-repo branch July 25, 2024 17:30
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