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

upgrade to openai-1.0 #16

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

drmikecrowe
Copy link

This PR migrates to OpenAI >= 1.0

NOTE: A lot of these changes are formatting issues. If you could do a ruff format on main, I can update and the diff will be cleaner.

For community

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@jpal91
Copy link
Owner

jpal91 commented Jun 13, 2024

Thanks for contributing! I was thinking about doing this a while back but got distracted, so this is great.

Also, I did just push the formatting to main, so if you want to merge the changes in, we should be good. Not sure why I never actually noticed that...

@drmikecrowe
Copy link
Author

I think the formatting took. LMK what you think

@drmikecrowe
Copy link
Author

Any chance to merge this soon?

Copy link
Owner

@jpal91 jpal91 left a comment

Choose a reason for hiding this comment

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

Hey, @drmikecrowe. Sorry I've been busy the last couple of weeks and haven't gotten around to this. I only noticed one issue that I made a comment on. After that we should be good to go.

tests/xontrib_chatgpt/test_chatgpt.py Outdated Show resolved Hide resolved
Copy link
Owner

@jpal91 jpal91 left a comment

Choose a reason for hiding this comment

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

Also just saw these indent issues as well. Should be an easy fix.

tests/xontrib_chatgpt/test_chatgpt.py Outdated Show resolved Hide resolved
tests/xontrib_chatgpt/test_chatgpt.py Outdated Show resolved Hide resolved
tests/xontrib_chatgpt/test_chatgpt.py Outdated Show resolved Hide resolved
@jpal91
Copy link
Owner

jpal91 commented Jul 18, 2024

One final thing...

Can you update the pyproject.toml to reflect the new dependency?

Just switch openai>=0.28.0 < 1 to openai>=1

@drmikecrowe
Copy link
Author

Done

Copy link
Owner

@jpal91 jpal91 left a comment

Choose a reason for hiding this comment

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

Hey @drmikecrowe. Just a couple more things I added in. Once everything's complete we should be fine. Let me know if you have any questions, though.

Also, make sure to checkout the indentation issue I mentioned above. Each of those statements just need to be indented after the with block(s) to get the tests to run properly.

.gitignore Outdated Show resolved Hide resolved
tests/xontrib_chatgpt/test_chatgpt.py Outdated Show resolved Hide resolved
@drmikecrowe
Copy link
Author

@jpal91 -- yeah, that was weird. Some kind of merge error duplicated some lines. All tests pass now, we should be good

@drmikecrowe
Copy link
Author

@jpal91 -- Should be ready to merge

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