-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use Pydantic-Settings for improved Config #9
base: main
Are you sure you want to change the base?
Conversation
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.
short review of changes that I did in addition to the switch to the conf
-pydantic settings.
# EDIT mmaelicke: | ||
# This file was in the original repo and hardcoded in the script. | ||
# I move this into the config for reasons of readability and clarity. | ||
# This file has the merged "megabasins_gdf" in it | ||
MERIT_BASINS_SHP: str = 'data/shp/basins_level2/merit_hydro_vect_level2.shp' |
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.
Note this change:
I added this path to the config as it was hard to spot before. On my system the data folder is located somewhere else, thus, I needed to config it.
merit_basins_shp = 'data/shp/basins_level2/merit_hydro_vect_level2.shp' | ||
megabasins_gdf = gpd.read_file(merit_basins_shp) | ||
#merit_basins_shp = 'data/shp/basins_level2/merit_hydro_vect_level2.shp' | ||
megabasins_gdf = gpd.read_file(conf.MERIT_BASINS_SHP) |
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 is the change I added to get rid of the hardcoded path in the middle of the script. Relates to my last comment
except Exception as e: | ||
raise Warning(f"Could not save pickle file to: {pickle_fname}. Message: {str(e)}") |
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.
here I changed the exception handling a bit to log out a message of the actual error.
Hey there,
Thanks for this great repo!
I rewrote parts to use pedantic-settings over global variables. This way, it's as easy as before to change the config, I also kept all config names as they were. However, with pydantic it is possible to create a config programmatically and even load from environment variables or .env files.
I need this as I run this code in a docker container and there is no easy way to change the config. These changes also help to use the repo additionally more like a library as importing is a bit easier this way.
I hope you like the changes.
Best,
Mirko