Skip to content

Use fun new exciting syntax#457

Closed
MattyTheHacker wants to merge 1 commit intomainfrom
magic
Closed

Use fun new exciting syntax#457
MattyTheHacker wants to merge 1 commit intomainfrom
magic

Conversation

@MattyTheHacker
Copy link
Copy Markdown
Member

No description provided.

@MattyTheHacker MattyTheHacker self-assigned this Apr 12, 2025
@MattyTheHacker MattyTheHacker added the enhancement New feature or request label Apr 12, 2025
@MattyTheHacker MattyTheHacker requested a review from Copilot April 12, 2025 17:58
Copy link
Copy Markdown
Contributor

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.

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

Copy link
Copy Markdown
Member

@CarrotManMatt CarrotManMatt left a comment

Choose a reason for hiding this comment

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

Not a big fan of this to be honest. It's very much just syntax preference and the balance is incredibly negligible. Overall I'd be a -0 on this syntax as it is slightly less readable in my personal opinion (goes against "Explicit is better than implicit." from The Zen Of Python.) It relies on the fact that the reader has a good grasp of Python's truthiness logic, and can present issues when the second value is of type T | None. Whereas the ternary operator keeps this explicit.

If this syntax was used as part of a new enhancement or bugfix PR I'd happily accept it's use, but won't include a change like this just for something I don't see as a major syntax improvement beyond personal opinion. (If I understand correctly these get compiled to functionally equivalent bytecode anyways.)

@CarrotManMatt CarrotManMatt added refactor Improvements to the codebase that do not directly affect users wontfix This will not be worked on and removed enhancement New feature or request labels Apr 12, 2025
auto-merge was automatically disabled April 12, 2025 19:08

Pull request was closed

@MattyTheHacker MattyTheHacker deleted the magic branch April 12, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Improvements to the codebase that do not directly affect users wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants