-
Notifications
You must be signed in to change notification settings - Fork 204
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
Update workflow to latest geopandas and fiona #1262
base: main
Are you sure you want to change the base?
Conversation
commit a220390 Author: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon Dec 23 19:39:49 2024 +0000 [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci commit 6bff954 Author: davide-f <67809479+davide-f@users.noreply.github.com> Date: Mon Dec 23 19:39:35 2024 +0000 Update pinned environment files for all platforms
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.
Looks good to me. As discussed, explicit specification of fiona
engine is needed only if schema
argument is applied (which basically correspond to a transformation of geopandas dataframe into geojson structure).
The only concern I have is save_to_geojson
in helpers
. It's used in clean_osm_data
and doesn't have specification of driver
and schema
argument in contrast to save_to_geojson
defined and used in build_shapes
and cluster_network
. Not sure about the implications, though.
pypsa-earth/scripts/_helpers.py
Lines 770 to 781 in fe41d4e
def save_to_geojson(df, fn): | |
if os.path.exists(fn): | |
os.unlink(fn) # remove file if it exists | |
# save file if the (Geo)DataFrame is non-empty | |
if df.empty: | |
# create empty file to avoid issues with snakemake | |
with open(fn, "w") as fp: | |
pass | |
else: | |
# save file | |
df.to_file(fn, driver="GeoJSON") |
Could you please check it just in case?
Otherwise, the PR looks great, and thanks a lot for tackling it! 😄
This PR has second priority with respect to #1172 . |
Closes # (if applicable).
Changes proposed in this Pull Request
It add a minor fix to enable the update of geopandas and fiona versions
Checklist
envs/environment.yaml
anddoc/requirements.txt
.config.default.yaml
andconfig.tutorial.yaml
.test/
(note tests are changing the config.tutorial.yaml)doc/configtables/*.csv
and line references are adjusted indoc/configuration.rst
anddoc/tutorial.rst
.doc/release_notes.rst
is amended in the format of previous release notes, including reference to the requested PR.