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(download): Fix download url by reverting change #197

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

thejaminator
Copy link
Contributor

@thejaminator thejaminator commented Jan 24, 2023

Fixes #196, and probably #185 and #183

This MR reverts the change in https://github.com/openai/openai-python/pull/146/files#r1084884786 so that the requests hit the /content url of the api again.

cc @Andrew-Chen-Wang @ddeville

To reproduce, on the older openai==0.25.0
`

openai api fine_tunes.results -i <job_id_here>

You'll get the output of the actual CSV file

E.g.

step,elapsed_tokens,elapsed_examples,training_loss,training_sequence_accuracy,training_token_accuracy
1,22048,32,0.011019098009307718,0.0,0.0
1,32288,32,0.007613447834811513,0.0,0.0

On the current openai==0.26.2

You get instead

{
"object": "file",
"id": "file-66NnpqAXzw3In27fROPIU03P",
"purpose": "fine-tune-results",
"filename": "compiled_results.csv",
"bytes": 386756,
"created_at": 1674504271,
"status": "processed",
"status_details": null
}

@Andrew-Chen-Wang
Copy link
Contributor

%s and f strings have different formatting rules, but good catch on the url path! Might've accidentally deleted it

@thejaminator
Copy link
Contributor Author

%s and f strings have different formatting rules, but good catch on the url path! Might've accidentally deleted it

interesting. whats the difference?

@hallacy
Copy link
Collaborator

hallacy commented Jan 24, 2023

(will merge once the above conversation is resolved)

@Andrew-Chen-Wang
Copy link
Contributor

Ref: https://stackoverflow.com/questions/5082452/string-formatting-vs-format-vs-f-string-literal

It's simply because of tuples and a few other things. (Take a look at flynt). I don't think it's applicable in this case in retrospect, so the change LGTM.

@hallacy hallacy merged commit 88b267b into openai:main Jan 25, 2023
davedittrich pushed a commit to davedittrich/openai-python that referenced this pull request Jan 27, 2023
Co-authored-by: James Chua <james@leadiq.com>
@stainless-bot stainless-bot mentioned this pull request Nov 6, 2023
cgayapr pushed a commit to cgayapr/openai-python that referenced this pull request Dec 14, 2024
Co-authored-by: James Chua <james@leadiq.com>
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.

Regression in download, openai api fine_tunes.results behavior
3 participants