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.

Sign up for GitHub

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

Jump to bottom

Revert Cardinality Requirement for Histograms #301

Merged
merged 23 commits into from
Mar 22, 2021
Merged

Revert Cardinality Requirement for Histograms #301

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2cef000
coalesce data_types into data_type_lookup
jinimukh Dec 23, 2020
cad3e84
Merge branch 'master' of https://github.com/lux-org/lux
jinimukh Dec 27, 2020
f197884
merge
jinimukh Jan 8, 2021
1ed9655
merge fixed
jinimukh Jan 8, 2021
c56f79d
merge conflicts
jinimukh Jan 8, 2021
c0388df
merged
jinimukh Jan 8, 2021
cf045de
Merge branch 'master' of https://github.com/jinimukh/lux
jinimukh Jan 8, 2021
6ae9767
Merge branch 'master' into foo
jinimukh Jan 8, 2021
0db6376
merge upstream
jinimukh Jan 9, 2021
1e6f572
first commit
jinimukh Jan 15, 2021
7836abf
conflicts
jinimukh Feb 20, 2021
9c17abb
requirements.txt updated for pandas 1.2.2
jinimukh Feb 21, 2021
b3509f6
Merge branch 'master' of https://github.com/jinimukh/lux
jinimukh Feb 21, 2021
d428289
Merge branch 'master' of https://github.com/lux-org/lux
jinimukh Mar 12, 2021
3e6389f
revert cardinality requiremment
jinimukh Mar 12, 2021
f6d344c
black reformat
jinimukh Mar 12, 2021
e32f50c
Merge branch 'master' of https://github.com/lux-org/lux into patch/un…
jinimukh Mar 21, 2021
33b0ad3
all tests passing with cardinality optimization
jinimukh Mar 21, 2021
cf947e0
remove abs value
jinimukh Mar 21, 2021
fce74a3
tests added
jinimukh Mar 21, 2021
f9d6b59
black reformat
jinimukh Mar 21, 2021
d9d39c2
minor fixes
dorisjlee Mar 22, 2021
853844a
black
dorisjlee Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions lux/action/univariate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ def univariate(ldf, *args):
ignore_rec_flag = False
if data_type_constraint == "quantitative":
possible_attributes = [
c
for c in ldf.columns
if ldf.data_type[c] == "quantitative" and ldf.cardinality[c] > 5 and c != "Number of Records"
c for c in ldf.columns if ldf.data_type[c] == "quantitative" and c != "Number of Records"
Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify, does ldf.cardinality[c] return the number of rows for a given column? In this case, I wonder if removing this check would lead to some irregular behavior when there are very few rows. Is there some other part in the code that checks for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

no. it returns the unique number of values -- we only compute histograms for dataframes > 5 rows.

]
intent = [lux.Clause(possible_attributes)]
intent.extend(filter_specs)
Expand All @@ -65,9 +63,7 @@ def univariate(ldf, *args):
ignore_rec_flag = True
elif data_type_constraint == "nominal":
possible_attributes = [
c
for c in ldf.columns
if ldf.data_type[c] == "nominal" and ldf.cardinality[c] > 5 and c != "Number of Records"
c for c in ldf.columns if ldf.data_type[c] == "nominal" and c != "Number of Records"
]
examples = ""
if len(possible_attributes) >= 1:
Expand All @@ -81,9 +77,7 @@ def univariate(ldf, *args):
}
elif data_type_constraint == "geographical":
possible_attributes = [
c
for c in ldf.columns
if ldf.data_type[c] == "geographical" and ldf.cardinality[c] > 5 and c != "Number of Records"
c for c in ldf.columns if ldf.data_type[c] == "geographical" and c != "Number of Records"
]
examples = ""
if len(possible_attributes) >= 1:
Expand Down
2 changes: 1 addition & 1 deletion lux/vislib/altair/Histogram.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def initialize_chart(self):

# Default when bin too small
if markbar < (x_range / 24):
markbar = (x_max - x_min) / 12
markbar = abs(x_max - x_min) / 12
Copy link
Member

Choose a reason for hiding this comment

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

Would there ever be a case such that calling abs is necessary? By construction, shouldn't x_max be greater than or equal to x_min?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, I added this as a final check for integer overflow so that at least our code doesn't error and it just means the user inputs are too extreme (positive or negative) to handle. @dorisjlee should we even worry about this?

Copy link
Member

Choose a reason for hiding this comment

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

I think xmax will always be larger than xmin.


self.data = AltairChart.sanitize_dataframe(self.data)
end_attr_abv = str(msr_attr.attribute) + "_end"
Expand Down
2 changes: 1 addition & 1 deletion tests/test_pandas_coverage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def test_transform(global_var):
df["Year"] = pd.to_datetime(df["Year"], format="%Y")
new_df = df.iloc[:, 1:].groupby("Origin").transform(sum)
new_df._repr_html_()
assert list(new_df.recommendation.keys()) == ["Correlation", "Occurrence"]
assert list(new_df.recommendation.keys()) == ["Correlation", "Distribution", "Occurrence"]
assert len(new_df.cardinality) == 7


Expand Down