-
Notifications
You must be signed in to change notification settings - Fork 115
Metadata and download modal - Date display (all charts) #5790
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
base: master
Are you sure you want to change the base?
Metadata and download modal - Date display (all charts) #5790
Conversation
…arts to the metadata modal. It covers the following tile types: bar, highlight, map, ranking and scatter. Shared functionality is provided in the TileUtils component. A note that while line chart (which already had this functionality applied in an earlier PR) has been updated to use the shared type, it does not use the shared utility function. This is because the line tile already gathered this date / statvar information in a way that we could make use of, a way that other tiles did not.
…on to calculate the stat var date range as the other charts. Although the line tile already had data that could be requisitioned for this, this update serves two purposes: it creates more cohesion between the two components (all the tiles calculate the ranges now in the same way) and two, it removes the O(nlogn) operation that the line chart was doing, replacing it with the O(n) operation the other charts were doing. It also simplifies the component somewhat.
…e now tying them to stat var / facet id combinations, because the charts sometimes display a combination of facets for the same stat var, and these may have different date ranges.
Summary of ChangesHello @nick-next, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request generalizes the display of chart-specific date ranges within the metadata and download dialogs across all chart types. It refines the underlying data structure for date tracking by incorporating facet information, which is crucial for accurately representing data from multiple sources or with varying granularities. This ensures that users receive precise date context for the data presented in any chart. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully extends the date display functionality to various chart types, including bar, scatter, map, ranking, and highlight tiles, building upon the previous work for line charts. A significant improvement is the update to StatVarFacetDateRangeMap to incorporate facet IDs, which correctly addresses scenarios where the same statistical variable might have different date ranges across various facets. The changes are consistently applied across components, ensuring that date ranges are accurately tracked and displayed in metadata and download modals. The introduction of the updateStatVarFacetDateRange utility function and the facetId in StatVarMetadata are key to this enhancement.
…teStatVarFacetDateRange` to use lodash to set and get the record. This latter update is in response to a github scanning comment concerning prototype pollution. In this case, it was safe to begin with because the source keys are trusted. However, to make it clear (and not have the function rely on upstream trust), I've switched to lodash setters, which handle this.
miss-o-soup
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Nick, I see now the displayed date. Thanks for adding this edit.
From I saw on the screenshots this looks good to me. Thanks!
dwnoble
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you nick! tested locally and verified it works for me. I appreciate how if i select a facet, and open the "About" modal before the facet finishes loading, that the modal will close itself and re-open once the facet does finish loading.
Description
This PR is a follow up to 5679. That PR introduced the display of the chart dates actually displayed into to the metadata modal and chart embed (download) dialogs for the line tile chart.
This PR follows that up with the date functionality applied to the remaining charts: bar, scatter, map, ranking and highlight.
Notes
In addition to applying the wiring of the dates from the remaining charts, this PR introduces an important update that also applies to the original line chart functionality.
We were originally maintaining a map of stat vars to date ranges. However, there are certain situation where the same stat var is populated via different facets (this can be seen often map charts, where, when “Span to date” is not selected, we often collate data for the same stat var across different facets with different dates).
Because of this, the original functionality of the first PR has been updated so that the mapping now includes the facet.
This can be seen in one of the screenshots below.
Testing
Most of the charts can be seen via this search:
Demographics in the United States
Scatter charts can be seen via this search:
Poverty vs Crime in US States
Screenshots
Highlight Tile
Bar
Graph / Ranking
(note the different dates for the same stat var - the 2023 date is Puerto Rico from a different data set)

Scatter
Download dialog