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.

Sign up for GitHub

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

Jump to bottom

Fix outdated multi-period usage docs #981

Merged
merged 5 commits into from
Oct 13, 2023
Merged

Fix outdated multi-period usage docs #981

merged 5 commits into from
Oct 13, 2023

Conversation

jokochems
Copy link
Member

  • Correct the type of periods attribute of an energy system to list.
  • Add hint to usage of timeincrement in combination with timeindex.
  • Provide minor doc fixes alongside.

@jokochems jokochems linked an issue Oct 2, 2023 that may be closed by this pull request
Outdated docs for use of multi-periods #978
Closed
@jokochems jokochems requested a review from henhuy October 2, 2023 14:09
@jokochems
Copy link
Member Author

@henhuy This should address the outdated docs. Thanks for pointing this out. Would you please review?

I really thought, I changed that in the first place after @p-snft pointed me to the possibility for code simplification. Seems like I either forgot or falsely commited to a wrong local branch that was never pushed back. Nevermind, now it should be correct.

henhuy
henhuy requested changes Oct 4, 2023
View reviewed changes
Copy link
Contributor

@henhuy henhuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor issue found.
Thanks for adapting docs!

src/oemof/solph/_energy_system.py Outdated Show resolved Hide resolved
@jokochems jokochems requested a review from henhuy October 6, 2023 17:07
henhuy
henhuy approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@henhuy henhuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the docs!

@jokochems
Copy link
Member Author

@p-snft since this only affects docs, all tests are succeeding and @henhuy has already reviewed, I'll merge this.

@jokochems jokochems merged commit 91c5b55 into dev Oct 13, 2023
14 checks passed
@p-snft p-snft deleted the docs/fix-outdated-multi-period-docs branch October 13, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@henhuy henhuy henhuy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Outdated docs for use of multi-periods
2 participants
@jokochems @henhuy