Skip to content

standardize query order_by argument#271

Merged
charliemirabile merged 1 commit intomasterfrom
order_by_desc
Sep 29, 2025
Merged

standardize query order_by argument#271
charliemirabile merged 1 commit intomasterfrom
order_by_desc

Conversation

@theyoyojo
Copy link
Contributor

@theyoyojo theyoyojo commented Aug 5, 2025

To sort the results of a query with peewee orm, in descending order,
we pass a column label to the order_by() method on a select() call
and either prefix it with the '-' symbol or call its.desc() method.

Currently we do some of both. Change all order_by({X.desc()} => {-X})
to make the code easier to read and more consistent.

Copy link
Contributor

@charliemirabile charliemirabile left a comment

Choose a reason for hiding this comment

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

I agree that we should standardize, but I think that there are only 2 existing instances of .desc() vs the 4 instances of - that you are changing here. I would argue that maybe we should standardize in the other direction. While terse, I personally find the operator overloading syntax that peewee offers for construction queries to actually be one of the main advantages of using an ORM.

To sort the results of a query with peewee orm, in descending order,
we pass a column label to the order_by() method on a select() call
and either prefix it with the '-' symbol or call its.desc() method.

Currently we do some of both. Change all order_by({X.desc()} => {-X})
to make the code easier to read and more consistent.

Signed-off-by: Joel Savitz <joel@underground.software>
Copy link
Contributor

@charliemirabile charliemirabile left a comment

Choose a reason for hiding this comment

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

lgtm

@charliemirabile charliemirabile merged commit c11605e into master Sep 29, 2025
1 check passed
@charliemirabile charliemirabile deleted the order_by_desc branch September 29, 2025 17:17
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