Skip to content

Conversation

@GLEF1X
Copy link

@GLEF1X GLEF1X commented Mar 26, 2023

No description provided.

GLEF1X added 2 commits March 26, 2023 14:33
* Remove unused variables and imports * Add some variables to `__all__` * Remove redundant parenthesis
@GLEF1X GLEF1X changed the title [deprecated]: remove invocations of obsolete methods [deprecated]: remove unused/deprecated code Mar 26, 2023
Copy link
Collaborator

@hallacy hallacy left a comment

Choose a reason for hiding this comment

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

Wow! Thank you. Thats a ton of little details. How did you find all of these?

rheaders,
)

def request_headers(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: can you check if the call in line 490 needs to get updated based on this?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, thank you for the review. It's indeed a bug that I overlooked. I've just fixed it in dc8feeb

@GLEF1X
Copy link
Author

GLEF1X commented Mar 30, 2023

Wow! Thank you. That's a ton of little details. How did you find all of these?

I was using a couple of linters, and It's a good idea to integrate some into this project. For instance, I saw a pretty good configuration in evals, so contributors can follow the same code style. Tools like pre-commit, mypy, isort or ruff can really help a lot.

@GLEF1X GLEF1X requested a review from hallacy March 30, 2023 13:30
@rattrayalex
Copy link
Collaborator

It's too bad this was never merged; the SDK has been entirely rewritten since.

stainless-app bot pushed a commit that referenced this pull request Mar 27, 2025
safa0 pushed a commit to safa0/openai-agents-python that referenced this pull request Apr 27, 2025
Towards openai#345 ## Summary: Using a `dict` or `Mapping` isn't strict-mode compliant. But we were checking for the literal `True` whereas the value can also be an array, for example. Fix that. ## Test Plan: Unit tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants