Skip to content

Conversation

@Herald-TUOS
Copy link
Contributor

No description provided.

@Herald-TUOS Herald-TUOS mentioned this pull request Sep 25, 2025
@Herald-TUOS Herald-TUOS self-assigned this Sep 25, 2025
@Herald-TUOS Herald-TUOS linked an issue Sep 25, 2025 that may be closed by this pull request
)
joined.update(nearest, overwrite=False)
joined["region_id"] = joined["region_id"].where(pd.notna(joined["region_id"]), None)
if "GSPGroup" in joined.columns:
Copy link
Contributor

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?

Copy link
Contributor Author

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

Comment on lines +285 to 286
return list(zip(joined["region_id"], joined["GSPGroup"]))
return joined["region_id"].tolist()
Copy link
Contributor

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)

Copy link
Contributor Author

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>
@Herald-TUOS
Copy link
Contributor Author

Any idea why these tests are failing @JamieTaylor-TUOS ?
They are succeeding locally.

(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

@Herald-TUOS Herald-TUOS merged commit 28ce520 into develop Oct 21, 2025
5 of 12 checks passed
@Herald-TUOS Herald-TUOS deleted the 61-return-gsps-and-gspgroups-after-reverse-geocoding branch October 21, 2025 12:43
@Herald-TUOS Herald-TUOS restored the 61-return-gsps-and-gspgroups-after-reverse-geocoding branch October 21, 2025 12:54
@Herald-TUOS Herald-TUOS deleted the 61-return-gsps-and-gspgroups-after-reverse-geocoding branch October 21, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Return GSPs and GSPGroups after reverse geocoding

3 participants