Issue227 - Turbine, Panel and CSP Installation Configs from local path

[LukasFrankenQ]

Closes #227.

Change proposed in this Pull Request

Methods get_windturbineconfig, get_solarpanelconfig and get_cspinstallationconfig load yaml files based on the passed
configuration name. Now, additionally, the methods recognize if a path to a locally stored config is passed.
The file is then loaded instead of a predefined config.

Description

The methods run Path([the local path]).exists() to check if the path exists. If not, the methods assume that the user passed
the name of a config, and proceeds as before the change.

Motivation and Context

This is a small change, proposed a while ago by @fneum. I seems like a sensible addition.

How Has This Been Tested?

I checked all three functions to make sure that both old and new behaviour are behaving as expected.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I tested my contribution locally and it seems to work fine.
• I locally ran pytest inside the repository and no unexpected problems came up. (one apparently unrelated error)
• I have adjusted the docstrings in the code appropriately.
• I have documented the effects of my code changes in the documentation doc/.
• I have added newly introduced dependencies to environment.yaml file.
• I have added a note to release notes doc/release_notes.rst.
• I have used pre-commit run --all to lint/format/check my contribution

[euronion]

Some ideas for improvements. What do you think?

NB: We need to be careful if we change how the of get_windturbineconfig is handled. If we only allow str values from atlite.resource.windturbines then that would be a breaking change which we should avoid.

[euronion]

Rather than presuming that if the Path does not exist that then it might be a pre-shipped one, we can check if it is a shipped config by checking:

turbine in windturbines

and then instead of manually constructing the path

turbine = windturbines[turbine]

as we have the turbines which are pre-shipped as well as their paths already read in windturbineconfig.

[euronion]

We need to do some adjustements here, as config.windturbine_dir does not exist (very old legacy piece of doc which shouldn't be here).

[euronion]

E.g.

The configuration can be either one of the configurations shipped with atlite, one loaded from a local path or retrieved from the Open Energy Database (OEDB):
If one of the names of the preshipped turbines from atlite.resources.windturbines is provided, that turbine configuration is loaded and returned.
If a pathlib.Path object is provided, this local path is read and the turbine configuration from this Path is parsed.
If a string starting with "oedb:" is provided, atlite will try to read the turbine configuration for this from OEDB.

[euronion]

See above for windturbine configs

[euronion]

See above for windturbine configs

[FabianHofmann]

Hey @LukasFrankenQ, this looks sensible! What is the state here? As far as I see the only thing missing is the doc suggestion of @euronion. I would not enforce a Path object, but allow Path and str (we have the same behavior for cutouts).

[LukasFrankenQ]

Hey @FabianHofmann, hey @euronion, thank you for the suggestions (and the suggestion by @euronion on Discord). I believe that the following strikes a balance between both suggestions made, i.e. retains flexibility wrt being agnostic in Path vs str, being short, being robust against obscure errors like passing turbine_name.yml instead of turbine_name.yaml.
The panel example would go

if isinstance(panel, str):
    try:
         panel = solarpanel[panel.replace('.yaml', '').replace('.yml', '')]
    except KeyError:
         pass

with open(panel, 'r') as f:
    conf = yaml.safe_load(f)

return conf

In case of the turbine it would of course first check for being an oedb file.

The only problem with this is that if I had nonexistent_turbine.yaml, I will get a FileNotFoundError which is harder to interpret than a KeyError, but I dont see a way around this given that we allow str objects to be both a turbine name and a path.
And then I would of course update the doc, what do you think?

[euronion]

I would prefer strongly enforcing Path for paths and only allowing str for turbine (and other) names @FabianHofmann .

We don't do it for cutouts partially due to legacy reasons and partially because we have the freedom to do so, as there are no "standard" cutouts which ship with atlite.

Standard use case in the past for the turbine name (and other configs) was to provide the name of the turbine which was translated into a path. Now we would just enforce this behaviour.

We could even think about omitting the .replace('.yaml', '').replace('.yml', '') from Lukas' proposal to make this even more clear: Either provide the name of the turbine or provide a path if you want to load a specific file.

[FabianHofmann]

@euronion you are right. I did not think properly about what the turbine argument can all indicate with this change. Enforcing a Path class for allowing to select a config file makes things clearer. Thanks for clarifying.

[LukasFrankenQ]

Hi @euronion, Hi @FabianHofmann,
Many thanks for the feedback and the suggestions, it is now all implemented. I also unified the docstrings, and think they are reasonable at explaining how to use the method.
Please let me know if there are any further requests!

[euronion]

Looking good, thanks for the PR!