Closed oscarsyu closed 3 years ago
@maxachis for your eyes only
Hi @oscarsyu,
This looks great! Like you said before, the conflict will be easy to handle bc your new version of the csv is the correct one. Just two fixes to check regarding address
and open_to_spec_group
:
address
--> I do not think you should add SITE1 + SITE2, instead it looks like if SITE2 is not NA, SITE2 info could be moved to the location_description
(also part of preparing the address field, line 45 and then 76/77 look redundant perhaps delete line 45)open_to_spec_group
# 1--> this is tough! nice job handling this. the thing I caught is in the source file, some of Population_Served_filter
say (everyone, student) and your result for open_to_spec_group
says only student. (see globalid
= DE34C305-4BD2-40A5-AE4F-21B8BC1B61A4 as an example). I would fix this so any field that contains everyone is considered everyone. open_to_spec_group
# 2 --> This is minor, but can you please set everyone = 0 instead of NA? 0 indicates it is NOT limited, while NA indicates we do not know. And this will align with other data sources. open_to_spec_group
# 3 --> Optional, but I think for any observation in this dataset that is not 0 (so some group), update the location_description
field to have the Population_Served information. And I would put it as the first field in location_description
(which I realize will get a bit long for this data source)I am going to close this pull request without merging it in. Once you make these fixes, please initiate a new pull request and we will be good to go!
Apologies for the late push!
I've added a script that fits the new GPCFB data in from Here
If someone could take a look at this and the produced csv and let me know if there are any edits to be made that would be wonderful. thanks!