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

NL: fix a few bugs #2945

Merged
merged 13 commits into from
Jul 14, 2023
Merged

NL: fix a few bugs #2945

merged 13 commits into from
Jul 14, 2023

Conversation

pradh
Copy link
Contributor

@pradh pradh commented Jul 14, 2023 •
edited
Loading

  1. Bug in extend_svs where we don't always check SV compatibility caused NL: should not show bar chart comparing very different SVs #2938
Screenshot 2023-07-13 at 7 54 21 PM
  1. Fix NL: bar chart with lots of variables is hard to read #2932 by limiting the number of extended SVs we plot in a bar chart to 16, and also keep the "main SV" pinned as the first entry so its clear.
Screenshot 2023-07-13 at 7 52 53 PM
  1. Fix NL: "electricity" recognized as CITY #2925 by ensuring that contained-in doesn't match substrings

pradh
pradh commented Jul 14, 2023
View reviewed changes
server/integration_tests/test_data/usa_map_types/query_2/chart_config.json
],
"title": "Median Income of White Households in Census Tracts of Placer County (${date})",
"title": "Median Income of Asian Households in Census Tracts of Placer County (${date})",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This and the next diff comes about because of the tweaks to the tracts maps in this PR. We consume away "tract" from the query and match more SVs I think...

chejennifer
chejennifer approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@chejennifer chejennifer left a comment

Choose a reason for hiding this comment

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

thanks for the fixes!

@pradh pradh enabled auto-merge (squash) July 14, 2023 16:57
@pradh pradh merged commit bddec6f into datacommonsorg:master Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chejennifer chejennifer chejennifer approved these changes

@shifucun shifucun shifucun approved these changes

@jehangiramjad jehangiramjad Awaiting requested review from jehangiramjad

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NL: bar chart with lots of variables is hard to read NL: "electricity" recognized as CITY
3 participants
@pradh @shifucun @chejennifer