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

Revert "Conditionally pass Hugging Face tokens for download when repo is private" #322

Closed
wants to merge 2 commits into from

Conversation

anth-volk
Copy link
Contributor

Reverts #321

Currently, in -api, the test at https://github.com/PolicyEngine/policyengine-api/actions/runs/12402725316/job/34625177069 fails because the API is unable to find the underlying dataset. Considering the timing and the fact that, when running locally using a previous version of -core, this test doesn't fail, I would imagine the cause of this problem is this code. Considering how important this code is, out of an abundance of caution, I am rolling back this code until we can determine the cause of the problem.

If experiencing the original issue (whereby, in a -us notebook, one doesn't have a token for Hugging Face, and is thus unable to run any code), a very simple workaround is to just hit enter when the token is requested. The code will work as expected, as the Hugging Face US repo is public.

@anth-volk anth-volk requested a review from MaxGhenis December 19, 2024 12:33
@anth-volk
Copy link
Contributor Author

Just a note: when running the failing test locally with the newest version of -core, the test does pass. I'm going to do a bit of digging to see if perhaps the issue is something related to the test, but it may still be best to merge this out of an abundance of caution.

@anth-volk
Copy link
Contributor Author

Turns out the issue is that it's just downloading to the wrong filepath, though I don't yet understand why this wasn't an issue in the previous implementation. I'm going to put this PR into draft. If I can't fix the issue by EOD today, I'll reopen and advocate reverting, then redoing. I think this will be easy enough to fix, though, so hoping to try today.

@anth-volk anth-volk marked this pull request as draft December 19, 2024 14:50
@anth-volk
Copy link
Contributor Author

Closing in favor of #325

@anth-volk anth-volk closed this Dec 19, 2024
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.

1 participant