datacommonsorg / website

Code for the Data Commons website
https://datacommons.org
Apache License 2.0
21 stars 74 forks source link

Add ILO topics #4430

Closed n-h-diaz closed 5 days ago

n-h-diaz commented 6 days ago

This reruns the core_topics script for ILO after updating the ILO schema in autopush (which mostly removes some variables)

n-h-diaz commented 6 days ago

Could you characterize what has changed in the description? Considering there are 37K additions and 116K descriptions.

Also, is it worth adding or updating this test:

https://github.com/datacommonsorg/website/blob/f2eb77b6051618989922cc5af01341372de33039/server/integration_tests/explore_test.py#L285-L295

Could you characterize what has changed in the description? Considering there are 37K additions and 116K descriptions.

Also, is it worth adding or updating this test:

https://github.com/datacommonsorg/website/blob/f2eb77b6051618989922cc5af01341372de33039/server/integration_tests/explore_test.py#L285-L295

Updated the description. Sorry could you clarify what you mean about the test? Should I change the ILO query?

dwnoble commented 6 days ago

Does this use the latest "undata_ilo_variable_grouping_rev2.csv" file? Can you check in the variable grouping changes as well?

dwnoble commented 5 days ago

Could you characterize what has changed in the description? Considering there are 37K additions and 116K descriptions. Also, is it worth adding or updating this test: https://github.com/datacommonsorg/website/blob/f2eb77b6051618989922cc5af01341372de33039/server/integration_tests/explore_test.py#L285-L295

Could you characterize what has changed in the description? Considering there are 37K additions and 116K descriptions. Also, is it worth adding or updating this test: https://github.com/datacommonsorg/website/blob/f2eb77b6051618989922cc5af01341372de33039/server/integration_tests/explore_test.py#L285-L295

Updated the description. Sorry could you clarify what you mean about the test? Should I change the ILO query?

For the two highlighted tests, they expect a chart config output like: https://github.com/datacommonsorg/website/blob/9e80cef75858d382b1888a8a392057d882e4423b/server/integration_tests/test_data/detection_api_undata_ilo_idx/chart_config.json#L4 and https://github.com/datacommonsorg/website/blob/9e80cef75858d382b1888a8a392057d882e4423b/server/integration_tests/test_data/detection_api_undata_dev_idx/chart_config.json#L4

respectively

I ran the tests locally (using ./run_test.sh --explore with an NL server running in a separate tab) and they worked, so i dont think they need updates.

@pradh should these tests be running on new pr pushes?

n-h-diaz commented 5 days ago

Does this use the latest "undata_ilo_variable_grouping_rev2.csv" file? Can you check in the variable grouping changes as well?

Updated to use the latest variable groupings file (and also added it to the PR)