Skip to content

WIP: Fixing issue where Optional[List[Enum]] didn't work, among others #61

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smt5541
Copy link
Collaborator

@smt5541 smt5541 commented Apr 23, 2025

🛠 Changes being made

Give examples of the changes you've made in this pull request. Include an itemized list if you can.

  • Add list_disable_query_csv Parameter argument, and corresponding FPV_LIST_DISABLE_QUERY_CSV config option for setting the default value
    • FPV_LIST_DISABLE_QUERY_CSV defaults to False to maintain backwards compatibility
    • This allows users to choose whether they want comma-separated values to be separated into lists
    • Disabling CSV splitting allows those parameter types to accept list[dict] and accommodates the possibility of a str or StrEnum containing a ,
  • Made generic type parsing (list, Union, Optional) recursive to enable support of list[Union[int, float]], Optional[list[int]], etc.
  • Updated README to note that the default Parameter argument doesn't handle None as a valid default because it's treated as no default set

🧠 Rationale behind the change

Why did you choose to make these changes?

Does this pull request resolve any open issues?

Were there any trade-offs you had to consider?

🧪 Testing

  • Have tests been added or updated for the changes introduced in this pull request?

  • Are the changes backwards compatible?

If the changes aren't backwards compatible, what other options were explored?

✨ Quality check

  • Are your changes free of any erroneous print statements, debuggers or other leftover code?

  • Has the README been updated to reflect the changes introduced (if applicable)?

💬 Additional comments

@smt5541 smt5541 self-assigned this Apr 23, 2025
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