Skip to content
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

Enabling Examples To Pass on Installation #1155

Closed
wants to merge 6 commits into from
Closed

Conversation

RobSisson
Copy link

  • Edited GenericStore instances to use nominal_storage_capacity
  • Edited Flow instances to use nominal_value
  • Removed duplicated "investment_nonconvex": False, in test_nodes_with_none_exclusion_old_name and test_nodes_with_none_exclusion in test_processing.py
  • Added openpyxl to optional dependencies (required for dispatch.py)
  • Updated the readme to include more comprehensive instructions

Suggestions and contributions were also made by https://github.com/RainerGaier

- Edited Flow instances to use nominal_value
- Removed duplicated "investment_nonconvex": False, in test_nodes_with_none_exclusion_old_name and test_nodes_with_none_exclusion in test_processing.py
@p-snft
Copy link
Member

p-snft commented Feb 16, 2025

Thanks again for your effort. I feel very sorry that we did not provide enough guidance and probably did duplicate work. Rainer already worked through the examples and I think we got them all fixed. If you still found issues, it might be because of problems in the presentation.

In fact, the naming nominal_capacity is for solph v0.6+, nominal_value for older versions. So, here you reverted a lot of changes that we already did for the upcoming release. For examples that target the current (at the moment v0.5) state of solph, you can look at https://oemof-solph.readthedocs.io/en/stable/examples/index.html or https://github.com/oemof/oemof-solph/tree/master (instead of https://github.com/oemof/oemof-solph/tree/dev).

@RobSisson
Copy link
Author

Ah I see! So my issue was that I was using examples which were for a newer version of OEMOF compared to the version of OEMOF I had installed?

@p-snft
Copy link
Member

p-snft commented Feb 17, 2025

Yes. The actual issue is that the example still tells (in the docstring) that it is for solph 0.5.x but it is for 0.6.x.

Now, I rely on your feedback: Is it really caused by the wrong docstring or didn't you read it anyway? If so, should we put something like assure_solph_version("0.6") into the executable code that will fail with a proper error message if run?

Just that I don't forget. A simple implementation would work like this:

def assure_solph_version(version_string: str) -> None:
"""
Tests given version string against the version of the presently installed version of solph.
"""

    solph_version = solph.__version__[:3]
    if solph_version != version_string:
        raise (RuntimeError(f"Unsupported version of solph: {solph_version}"))

assure_solph_version("0.6")

@p-snft p-snft marked this pull request as draft February 17, 2025 16:37
@p-snft
Copy link
Member

p-snft commented Feb 17, 2025

It's cleaner to continue at a separate issue (#1156). I will close this.

@p-snft p-snft closed this Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants