Features/#59 move resource management to importlib resources #119
Conversation
| @@ -12,16 +12,25 @@ | |||
| import egon.data.processing.openstreetmap as process_osm | |||
| import egon.data.importing.zensus as import_zs | |||
|
|
|||
| from importlib_resources import files | |||
There was a problem hiding this comment.
This is a third party library import. According to "CONTRIBUTING.rst" this should be placed in a section between builtin imports and package local imports. But I see that others messed up the import order before you already.
Also, the importlib-resources package should be added to the depen dencies in "setup.py".
| print('*-*-*-*-*-*') | ||
| print(os.path.abspath(os.path.join(os.path.dirname( | ||
| __file__ ), '..', '..', 'processing', 'vg250'))) | ||
| print(files('egon.data.processing').joinpath('vg250')) |
There was a problem hiding this comment.
Since you removed those changes in a later commit, I think you committed them by accident and know that such debug statements usually shouldn't be part of published commits. But now that I see how you used those statements, let me point out, that you could also have tried them out in an interactive console session, obviating the need to write them into a file in the first place.
src/egon/data/cli.py
Outdated
| @@ -85,7 +88,9 @@ def serve(context): | |||
| @click.version_option(version=egon.data.__version__) | |||
| @click.pass_context | |||
| def main(context, **kwargs): | |||
| os.environ["AIRFLOW_HOME"] = os.path.dirname(egon.data.airflow.__file__) | |||
| #os.environ["AIRFLOW_HOME"] = os.path.dirname(egon.data.airflow.__file__) | |||
| os.environ["AIRFLOW_HOME"] = os.path.dirname(files(egon.data.airflow).joinpath('__init__.py')) | |||
There was a problem hiding this comment.
This line exceeds the 79 character limit. Also os.path.dirname just removes the '__init__.py' component, so files(egon.data.airflow) should suffice.
There was a problem hiding this comment.
Using the files(egon.data.airflow) yields TypeError: str expected, not PosixPath. So, I changed it to str(files(egon.data.airflow)). I hope adding str() is ok.
| os.path.dirname(__file__), osm_config["target"]["path"] | ||
| #os.path.dirname(__file__), | ||
| files(import_openstreetmap), | ||
| osm_config["target"]["path"] |
There was a problem hiding this comment.
There's no reason to split up the original line. Our automatic code formatting tool will join them again anyway.
| os.path.dirname(__file__), osm_config["target"]["path"] | ||
| #os.path.dirname(__file__), | ||
| files(import_openstreetmap), | ||
| osm_config["target"]["path"] |
There was a problem hiding this comment.
Will also be joined when formatting code.
src/egon/data/db.py
Outdated
| @@ -16,7 +17,8 @@ def credentials(): | |||
| Complete DB connection information | |||
| """ | |||
| # Read database configuration from docker-compose.yml | |||
| package_path = egon.data.__path__[0] | |||
| #package_path = egon.data.__path__[0] | |||
| package_path = files('egon.data') | |||
There was a problem hiding this comment.
When the package has been imported, you can also just write files(egon.data), guarding against typos.
src/egon/data/config.py
Outdated
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| from importlib_resources import files | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
This style change is correct, but should be done in a separate commit. Ideally, style changes should be separate from changes modifying behaviour.
| #os.path.abspath(os.path.join(os.path.dirname( | ||
| # __file__ ), '..', '..', 'processing', 'vg250')) | ||
| files('egon.data.processing').joinpath('vg250') | ||
|
|
There was a problem hiding this comment.
I think files('egon.data.processing.vg250') should do the same.
…buses Fixes/#119 isolated ch4 foreign buses
It is intended to address the issues#59 'Move resource management to
importlib.resources'