-
Notifications
You must be signed in to change notification settings - Fork 1
return GSPGroup while reverse-geocoding to GSP
#62
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
return GSPGroup while reverse-geocoding to GSP
#62
Conversation
| ) | ||
| joined.update(nearest, overwrite=False) | ||
| joined["region_id"] = joined["region_id"].where(pd.notna(joined["region_id"]), None) | ||
| if "GSPGroup" in joined.columns: |
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.
Not sure what the if condition is for? won't this always evaluate to True?
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.
This won't be True when reverse geo-coding llsoa and nuts
| return list(zip(joined["region_id"], joined["GSPGroup"])) | ||
| return joined["region_id"].tolist() |
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.
I don't think its desirable to sometime return a list of strings and other times to return a list of tuples. I think we should always return a list of tuples, or else introduce a parameter to determine whether to return the GSPGroup or not (though the latter smells a little)
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.
Since llsoa and nuts doesn't need to return a list of tuples, I think we should come up with something else. Is it okay to make a separate function for reverse geo-coding gsp?
Co-authored-by: Jamie Taylor <jamie.taylor@sheffield.ac.uk>
|
Any idea why these tests are failing @JamieTaylor-TUOS ? (geocode) ph1ho@phy13076:~/Documents/Geocode$ git checkout 61-return-gsps-and-gspgroups-after-reverse-geocoding
Switched to branch '61-return-gsps-and-gspgroups-after-reverse-geocoding'
Your branch is up-to-date with 'origin/61-return-gsps-and-gspgroups-after-reverse-geocoding'.
(geocode) ph1ho@phy13076:~/Documents/Geocode$ pytest Tests/
=================================================================================== test session starts ====================================================================================
platform linux -- Python 3.10.12, pytest-8.3.4, pluggy-1.5.0
rootdir: /home/ph1ho/Documents/Geocode
plugins: requests-mock-1.11.0, anyio-4.8.0, cov-6.0.0, mock-3.14.0
collected 9 items
Tests/test_geocode.py ......... [100%]
===================================================================================== warnings summary =====================================================================================
Tests/test_geocode.py::geocodeTestCase::test_geocode_postcode
/home/ph1ho/Documents/Geocode/geocode/cpo.py:163: FutureWarning: The provided callable <function nanmean at 0x77a1d6351e10> is currently using SeriesGroupBy.mean. In a future version of pandas, the provided callable will be used directly. To keep current behavior pass the string "mean" instead.
results_.groupby("id")
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
========================================================================= 9 passed, 1 warning in 126.35s (0:02:06) =========================================================================
(geocode) ph1ho@phy13076:~/Documents/Geocode$ python3 --version
Python 3.13.2
|
No description provided.