Skip to content
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

Specify the metric to order by for Series Limit #1351

Merged
merged 1 commit into from
Oct 18, 2016

Conversation

mistercrunch
Copy link
Member

Used to be based on the first metric from the metric multi-choice, but there's the use case where you don't want to show the metric you are sorting by. It will still default to use the first metric if the new select is empty

@mistercrunch mistercrunch force-pushed the specify_order_limit branch 2 times, most recently from 842b1a7 to bc43be3 Compare October 14, 2016 21:16
@NeginNejati
Copy link

NeginNejati commented Oct 14, 2016

👍 Thanks a lot Maxime. Your very quick help was well above my expectation :)

subq = subq.order_by(desc(main_metric_expr))
ob = main_metric_expr
if timeseries_limit_metric_expr is not None:
ob = timeseries_limit_metric_expr

Choose a reason for hiding this comment

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

same here.

timeseries_limit_metric_expr = None
if timeseries_limit_metric:
timeseries_limit_metric_expr = \
timeseries_limit_metric.sqla_col

Choose a reason for hiding this comment

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

minor: how about ternary?

Copy link
Member Author

Choose a reason for hiding this comment

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

too long for PEP8's 80 char limit

@@ -1689,6 +1699,7 @@ def query( # druid
filter=None, # noqa
is_timeseries=True,
timeseries_limit=None,
timeseries_limit_metric=None,

Choose a reason for hiding this comment

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

umm why do we need to override this here?

timeseries_limit_metric = metrics_dict.get(timeseries_limit_metric)
timeseries_limit_metric_expr = None
if timeseries_limit_metric:
timeseries_limit_metric_expr = \

Choose a reason for hiding this comment

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

just curious, what's \ at the end? (am new to python)

Copy link
Member Author

Choose a reason for hiding this comment

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

python's way of terminating a line on another line

Choose a reason for hiding this comment

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

👍

@mistercrunch mistercrunch merged commit ecb951b into apache:master Oct 18, 2016
@mistercrunch mistercrunch deleted the specify_order_limit branch October 18, 2016 01:34
neevany pushed a commit to Kieraya/caravel that referenced this pull request Oct 24, 2016
@github35
Copy link

@mistercrunch Hi, do you have any plan on enabling metrics ordering? For example, I want to show the first metric on the left in the visualization, but it is always the largest on the left. Just asking... I know you have to prioritize everything. Thanks a lot!

@pawan-kv
Copy link

How to specify the order by on metrics (computed columns) ?
For ex. if i want to show only top rows after order by "particular_metric asc". What should i do. It looks like superset/caravel always sort in "desc". How to change that ?. Theres no field for that.

@dpgaspar dpgaspar mentioned this pull request Apr 30, 2020
12 tasks
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 17, 2021
…pache#1351)

* feat: add x and y label support for timeseries

fix apache#16512

* feat: change label to title

* feat: add x/y title to 9 chats

* refactor: move config to shared-control

* refactor: add chartTitle section config in custom tab

* refactor: refactor param names

* lint: code lint

* refactor: refactor code

* lint: lint code

* feat: make ypostion clearable false

* lint: code lint

Co-authored-by: xuzhebin <maxhui2020@gmail.com>
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 24, 2021
…pache#1351)

* feat: add x and y label support for timeseries

fix apache#16512

* feat: change label to title

* feat: add x/y title to 9 chats

* refactor: move config to shared-control

* refactor: add chartTitle section config in custom tab

* refactor: refactor param names

* lint: code lint

* refactor: refactor code

* lint: lint code

* feat: make ypostion clearable false

* lint: code lint

Co-authored-by: xuzhebin <maxhui2020@gmail.com>
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 25, 2021
…pache#1351)

* feat: add x and y label support for timeseries

fix apache#16512

* feat: change label to title

* feat: add x/y title to 9 chats

* refactor: move config to shared-control

* refactor: add chartTitle section config in custom tab

* refactor: refactor param names

* lint: code lint

* refactor: refactor code

* lint: lint code

* feat: make ypostion clearable false

* lint: code lint

Co-authored-by: xuzhebin <maxhui2020@gmail.com>
zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 26, 2021
…pache#1351)

* feat: add x and y label support for timeseries

fix apache#16512

* feat: change label to title

* feat: add x/y title to 9 chats

* refactor: move config to shared-control

* refactor: add chartTitle section config in custom tab

* refactor: refactor param names

* lint: code lint

* refactor: refactor code

* lint: lint code

* feat: make ypostion clearable false

* lint: code lint

Co-authored-by: xuzhebin <maxhui2020@gmail.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.12.0 labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants