Skip to content

Conversation

uvchik
Copy link
Member

@uvchik uvchik commented Nov 19, 2021

No description provided.

@uvchik uvchik self-assigned this Nov 19, 2021
@uvchik uvchik force-pushed the revision/use-typical-oemof-template branch from 11faa9f to 4f3d7d7 Compare November 19, 2021 13:12
@pep8speaks
Copy link

pep8speaks commented Nov 19, 2021

Hello @uvchik! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-11-24 11:42:36 UTC

@uvchik uvchik added this to the v0.1.9 milestone Nov 19, 2021
@uvchik
Copy link
Member Author

uvchik commented Nov 19, 2021

I am not sure where to locate the examples but everything else looks fine to me.

@uvchik uvchik requested a review from p-snft November 19, 2021 15:52
@uvchik
Copy link
Member Author

uvchik commented Nov 19, 2021

I am also not sure if appveyor-tests for Windows are necessary but I implemented some.

Copy link
Member

@p-snft p-snft left a comment

Choose a reason for hiding this comment

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

I also don't know where to put the examples. However, we are anyway not consistent within oemof.

for key in ann_el_demands_per_sector:
assert np.isclose(demands[key], ann_el_demands_per_sector[key])
matplotlib.use("Agg")
import demandlib_examples.power_demand_example # noqa: F401
Copy link
Member

Choose a reason for hiding this comment

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

The import might incidentally do the correct thing, but exec(open("filename.py").read()) should be the clean way. Alternatively, the example could have a function that can be imported and called from her. (And if __name__ == "__main__": in the example script file.)

Copy link
Member

Choose a reason for hiding this comment

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

exec(...) will change the way file is interpreted. Thus, the test script for the examples has to be in the same location the examples are. (Alternatively, only function in the test scrips are tested but not the full scripts.)

p-snft and others added 3 commits November 23, 2021 11:16
Also, all examples in the corresponding directory should be tested
just by being in that directory and ending "_example.py".
@uvchik
Copy link
Member Author

uvchik commented Nov 23, 2021

I fixed the tests after you moved the examples. Everything fine now?


# read example temperature series
datapath = os.path.join(os.path.dirname(__file__), "example_data.csv")
datapath = os.path.join(os.getcwd(), "example_data.csv")
Copy link
Member

Choose a reason for hiding this comment

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

This will not work if people try to "python ./examples/heat_demand_example.py". But this is probably acceptable.

@uvchik uvchik requested a review from p-snft November 24, 2021 12:06
@uvchik uvchik merged commit 0902286 into dev Nov 25, 2021
@uvchik uvchik deleted the revision/use-typical-oemof-template branch November 25, 2021 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants