-
Notifications
You must be signed in to change notification settings - Fork 2
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
Great Big Green Week 2024 import #607
Conversation
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.
One bug, I think, and a couple of comments about whether there’s anything simple we can do to better document the format and necessity of the --year
flag on generate_gbgw_data.py
.
def add_arguments(self, parser): | ||
super().add_arguments(parser) | ||
|
||
parser.add_argument("--year", action="store", help="The year for the events") |
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.
Is there any way to make it raise a more obvious error, when you inevitably forget to provide a --year
flag? Currently, you get a traceback ending with:
…
File "/usr/local/lib/python3.9/site-packages/pandas/io/parsers/readers.py", line 1735, in _make_engine
self.handles = get_handle(
File "/usr/local/lib/python3.9/site-packages/pandas/io/common.py", line 856, in get_handle
handle = open(
FileNotFoundError: [Errno 2] No such file or directory: '/app/data/gbgw_events_None.csv'
Perhaps required=True
would do it?
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.
Also, is this meant to be a full year, like 2024
, or just the last two digits, like 24
? Line 13 of your import_gbgw_events_24.py
suggests the latter?
message = "Importing 2024 GBGW events" | ||
uses_gss = False | ||
|
||
data_file = settings.BASE_DIR / "data" / "gbgw_events_24_processed.csv" |
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.
See my comment from generate_gbgw_data.py – should this be gbgw_events_24_processed.csv
or gbgw_events_2024_processed.csv
?
|
||
data_file = settings.BASE_DIR / "data" / "gbgw_events_24_processed.csv" | ||
|
||
area_types = ["WMC", "WMC23", "STC", "DIS"] |
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.
When I generate gbgw_events_24_processed.csv
, the WMC
column is empty. I assume this is intentional, because geocoding a lonlat to pre-2024 constituencies with MapIt is a pain.
So I think you need to remove "WMC"
from the area_types
here, so that the base importer doesn’t try to create data for (pre-2024) WMCs.
If you leave "WMC"
in this list, without a column of WMCs in gbgw_events_24_processed.csv
, you’ll get this error:
File "/app/hub/management/commands/base_importers.py", line 81, in get_cons_col
return self.cons_col_map[self.area_type]
KeyError: 'WMC'
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.
💥 BOOM! Love it. LGTM!
cfaf8f8
to
601b0ca
Compare
Add a generic processing command to turn the spreadsheet of events into one with constituencies. Add a new command for the 2024 events. Fixes #593
merged in 601b0ca |
Add a generic processing command to turn the spreadsheet of events into one with constituencies. Add a new command for the 2024 events.
Fixes #593