Skip to content

Conversation

DA-344
Copy link
Contributor

@DA-344 DA-344 commented Sep 10, 2025

Summary

Option did not correctly resolve the type of an annotation when it was typed like param: 'ForwardRefToSomeThing' or when using from __future__ import annotations.

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

@DA-344 DA-344 requested a review from a team as a code owner September 10, 2025 18:15
@pycord-app
Copy link

pycord-app bot commented Sep 10, 2025

Thanks for opening this pull request!
Please make sure you have read the Contributing Guidelines and Code of Conduct.

This pull request can be checked-out with:

git fetch origin pull/2919/head:pr-2919
git checkout pr-2919

This pull request can be installed with:

pip install git+https://github.com/Pycord-Development/pycord@refs/pull/2919/head

@pullapprove4 pullapprove4 bot requested review from plun1331 and Dorukyum September 10, 2025 18:15
@DA-344 DA-344 force-pushed the fix/options-annotations branch from 404889a to 694de67 Compare September 10, 2025 18:15
@DA-344 DA-344 requested review from a team as code owners September 10, 2025 18:15
Signed-off-by: DA344 <108473820+DA-344@users.noreply.github.com>
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

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

This PR changes out-of-scope parts and removes certain things

@Soheab Soheab requested a review from Copilot September 19, 2025 12:55
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where the Option class in Discord commands did not correctly resolve type annotations when they were provided as string literals or forward references (e.g., param: 'ForwardRefToSomeThing' or when using from __future__ import annotations).

  • Adds support for resolving string-based type annotations using a new resolve_annotation utility
  • Improves parameter validation by adding proper type hints and using iterators more efficiently
  • Updates import statements to include necessary utilities and remove unused imports

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
discord/commands/core.py Adds string annotation resolution logic, improves type hints, and cleans up imports
CHANGELOG.md Documents the bug fix for string/forward reference annotation parsing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


final_options = []
for p_name, p_obj in params:
cache = {}
Copy link
Preview

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The cache is created fresh for each call to _parse_options, which means annotation resolution results won't be cached across different command parsing operations. Consider moving the cache to a higher scope (instance or class level) to improve performance for repeated annotation resolutions.

Copilot uses AI. Check for mistakes.

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.

4 participants