Allow different sorters to work with Vis Object by jinimukh · Pull Request #350 · lux-org/lux

jinimukh · 2021-04-09T03:56:50Z

Overview

This PR introduces a way to incorporate different sorters for Vis Objects. Currently, this only includes interestingness and alphabetical sorts, but later on, we can extend this class.

Changes

-- add sorters.py in utils
-- add lux.config.sorter in config
-- add tests in test_config

Example Output

TBD

codecov · 2021-04-09T04:04:33Z

Codecov Report

Merging #350 (d08c17c) into master (623e5f3) will increase coverage by 0.30%.
The diff coverage is 91.42%.

@@            Coverage Diff             @@
##           master     #350      +/-   ##
==========================================
+ Coverage   65.94%   66.24%   +0.30%     
==========================================
  Files          55       56       +1     
  Lines        4416     4468      +52     
==========================================
+ Hits         2912     2960      +48     
- Misses       1504     1508       +4

Impacted Files	Coverage Δ
lux/utils/orderings.py	`84.61% <84.61%> (ø)`
lux/_config/config.py	`77.27% <100.00%> (+1.72%)`	⬆️
lux/action/correlation.py	`86.66% <100.00%> (ø)`
lux/action/enhance.py	`100.00% <100.00%> (ø)`
lux/action/filter.py	`91.66% <100.00%> (ø)`
lux/action/temporal.py	`96.61% <100.00%> (ø)`
lux/action/univariate.py	`89.13% <100.00%> (ø)`
lux/vis/VisList.py	`51.04% <100.00%> (-0.26%)`	⬇️
lux/vislib/altair/ScatterChart.py	`96.96% <0.00%> (+3.03%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 623e5f3...d08c17c. Read the comment docs.

dorisjlee · 2021-04-10T23:48:19Z

lux/_config/config.py

 from typing import Any, Callable, Dict, Iterable, List, Optional, Union
 import lux
 import warnings
+from lux.utils.sorters import Sorter


Can we rename Sorter to something more specific? Currently, the sorter actually doesn't perform any sorting, but just returns the sort key, e.g. interestingness score, or sort key for alphabetical sorting.

dorisjlee · 2021-04-10T23:49:24Z

lux/_config/config.py

                stacklevel=2,
            )

+    @property


Not sure if sort makes sense as a global option, since it is more common to do one type of sorting for an action, but a different sort mechanism for another action.

I added two implementations -- one global ordering (this could be useful for sorting by attribute name or something really general that you want to standardize across all actions) and an action-wise ordering dictionary. If no ordering is defined for the specific action, then we fallback to the global ordering.

This sounds great, let's add some docs that explain the two different types of ways to specify sorting!

lux/vis/VisList.py

lux/utils/sorters.py

…/sort_interface

dorisjlee · 2021-08-24T17:55:07Z

tests/test_config.py

+    for vis in df.recommendation["Correlation"]:
+        assert vis.get_attr_by_channel("y")[0].attribute <= string
+        string = vis.get_attr_by_channel("y")[0].attribute
+    lux.config.sorter = "interestingness"


What is the difference between sorter and ordering?

dorisjlee · 2021-08-24T17:57:04Z

lux/utils/orderings.py

+            return Ordering.interestingness
+        elif value == "title":
+            return Ordering.title
+        elif value == "x_attr":


can we make "title", "x_attr" and "y_attr" more readable? e.g.) "alphabetical_by_x", "alphabetical_by_y", "alphabetical_by_title"

dorisjlee · 2021-08-24T17:57:48Z

lux/_config/config.py

                stacklevel=2,
            )

+    @property


This sounds great, let's add some docs that explain the two different types of ways to specify sorting!

dorisjlee · 2021-08-24T18:00:31Z

tests/test_config.py

+def test_ordering_actions(global_var):
+    lux.config.topk = 5
+
+    lux.config.ordering_actions["correlation"] = "x_attr"


The keys to "ordering_actions" is lowercase, would it break if the users specify uppercase ["Correlation"] instead?

dorisjlee · 2021-09-28T04:44:26Z

lux/_config/config.py

+        return self._ordering
+
+    @ordering.setter
+    def ordering(self, value):


Can we rename all ordering as ranking? We should avoid the word sorter in places where it might be unclear what it is referring it.

dorisjlee · 2021-09-28T04:45:54Z

tests/test_config.py

 import pandas as pd
 import time
 from lux.vis.VisList import VisList
+from lux.utils.orderings import Ordering


Change orderings to ranking or sorter, it is more clear if we are consistent across the terminology used.

I'll go for ranking because sort order (i.e. ascending/descending) uses "sort".

dorisjlee · 2021-09-28T04:46:31Z

tests/test_config.py

+    )
+
+    for vis in df.recommendation["Correlation"]:
+        assert (


For every assert, we should add the error message to be thrown if the assertion does not pass.

dorisjlee · 2021-09-28T04:49:08Z

doc/source/advanced/sorting.rst

+    def sort_by_multiple(collection, desc):
+        collection.sort(key=lambda x: (x.get_attr_by_channel("x")[0].attribute, x.get_attr_by_channel("y")[0].attribute), reverse=False)
+    lux.config.ordering = sort_by_multiple
+


We didn't explain how lux.config.ordering can be used in the default case.

jinimukh added 20 commits December 22, 2020 20:55

coalesce data_types into data_type_lookup

2cef000

Merge branch 'master' of https://github.com/lux-org/lux

cad3e84

merge

f197884

merge fixed

1ed9655

merge conflicts

c56f79d

merged

c0388df

Merge branch 'master' of https://github.com/jinimukh/lux

cf045de

Merge branch 'master' into foo

6ae9767

merge upstream

0db6376

first commit

1e6f572

conflicts

7836abf

requirements.txt updated for pandas 1.2.2

9c17abb

Merge branch 'master' of https://github.com/jinimukh/lux

b3509f6

Merge branch 'master' of https://github.com/lux-org/lux

d428289

Merge branch 'master' of https://github.com/lux-org/lux

9716088

Merge branch 'master' of https://github.com/lux-org/lux

912f3cc

Merge branch 'master' of https://github.com/lux-org/lux

8041f84

config sorters implemented; tests passing when run together

9548b8a

should fix failing interestingness tests

e5f5aa4

black

0aa92a2

dorisjlee requested changes Apr 10, 2021

View reviewed changes

jinimukh added 5 commits August 11, 2021 11:12

Merge branch 'master' of https://github.com/lux-org/lux into features…

0b8ee08

…/sort_interface

structural changes, some refactoring

31ec5a6

black

5aa2178

bug

bd6d4b4

add action specific sorting

79e4a41

dorisjlee requested changes Aug 24, 2021

View reviewed changes

jinimukh added 2 commits September 22, 2021 08:27

tests working

f9e34bc

documentation

ecdd3ea

jinimukh force-pushed the features/sort_interface branch from dd380ca to ecdd3ea Compare September 22, 2021 16:57

jinimukh and others added 2 commits September 22, 2021 09:58

Merge branch 'master' into features/sort_interface

0141fa1

fix bug

d08c17c

dorisjlee requested changes Sep 28, 2021

View reviewed changes

Conversation

jinimukh commented Apr 9, 2021

Overview

Changes

Example Output

Uh oh!

codecov bot commented Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Apr 9, 2021 •

edited

Loading