Revision locate bus #1195
Revision locate bus #1195
Conversation
There was a problem hiding this comment.
Brilliant work @davide-f!!
Have gone trough the changes and added some comments which are mainly quite minor. The only potentially crucial point is changing arguments of mock_snakemake, but I assume there has been a reason for that. All the other points can be also addressed in future PRs if you feel that improvements would take too much time for now.
Thanks a lot for investigating that, and looking forward to see CI running! 😄
| col = "name" | ||
| if not gadm_clustering: | ||
| gdf_shapes = gpd.read_file(path_to_gadm) | ||
| else: | ||
| if path_to_gadm: | ||
| gdf_shapes = gpd.read_file(path_to_gadm) | ||
| if "GADM_ID" in gdf_shapes.columns: | ||
| col = "GADM_ID" |
There was a problem hiding this comment.
Not sure it's really easy this part with four nested if-conditions. Agree that it's not the point to be addressed now, as the fix is quite urgent, but could we probably add a TODO for that?
There was a problem hiding this comment.
Completely agree; this is to maintain the features of the previous code.
To revise this I believe there is the need to revise this together with the revision of the alternative clustering procedure.
Good to add a TODO
| df = df[df.country.isin(countries)] | ||
| df[col_out] = None | ||
| for co in countries: | ||
| gdf_shape, col = _get_shape_col_gdf( |
There was a problem hiding this comment.
I recognise that it may be opening Pandora box, but wouldn't it be possible to replace naming for df and col with something a bit more specific? To my perception, it could probably facilitate a bit reading of the code. What do you think?
| Only cement not steel - proof of concept. | ||
| Change hotmaps to more descriptive name, etc. | ||
| """ |
There was a problem hiding this comment.
A docstring of map_industry_to_buses( ) seems to be a bit outdated. May it be a good idea to fix it?
Closes # (if applicable).
Changes proposed in this Pull Request
This PR proposes a revision of the procedure used in locate_bus and adopt geospatial comparisons of geopandas. This methodology is heavier and has deep code implications; therefore, I quickly merged a fix to solve the CI first.
A similar approach can be also included into build_powerplants and cluster_network.
Currently, it is adopted in the sector scripts where more outliers have been founded
Checklist
envs/environment.yamlanddoc/requirements.txt.config.default.yamlandconfig.tutorial.yaml.test/(note tests are changing the config.tutorial.yaml)doc/configtables/*.csvand line references are adjusted indoc/configuration.rstanddoc/tutorial.rst.doc/release_notes.rstis amended in the format of previous release notes, including reference to the requested PR.