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

Simpler Test Data #110

Merged
merged 65 commits into from
Aug 13, 2024
Merged

Simpler Test Data #110

merged 65 commits into from
Aug 13, 2024

Conversation

meksor
Copy link
Contributor

@meksor meksor commented Aug 7, 2024

Decided to remove all the bespoke test data generation and replace it with a kind-of standardized "fixtures" folder.
Screenshot from 2024-08-12 14-57-22
The fixtures for the benchmarks are now also in the git repo and can be run by anyone without additional work.
The tests can now also run tests for different platform types, this enables us to disable a few hundred tests which are currently run even though we dont need them (f.e. sqlite runs in the postgres v15 tests).
I've configured the github action so all the test runs are run inparalell so PRs should be “✓” faster in the future.
Running in parallel actually seems to add significant overhead to the amount of minutes used in total (about double)

@meksor
Copy link
Contributor Author

meksor commented Aug 12, 2024

@danielhuppmann @glatterf42 Seems this change has a conflict with a recent PR from you two, how should we resolve this. Are both of you okay with the new way of creating test data and should we migrate the changes from that PR here?

@meksor
Copy link
Contributor Author

meksor commented Aug 12, 2024

I've resolved the merge conflicts for now, but it looks like these utility data-creation functions make the tests quite complex imo.
I would recommend @glatterf42 to migrate the create_indexsets_for_run function to a csv-based data fixture in a follow up PR as well.

@glatterf42
Copy link
Member

Just so its on the top level as well: Thanks for the suggestions, you can make multiline-suggestions by selecting multiple lines and commenting a "suggestion" code block. btw:

Yes, thanks, I know. For some reason, Github didn't let me do that as it usually does this time 🤷

Copy link
Member

@glatterf42 glatterf42 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 addressing my feedback. There are some more minor changes I would like to see (mostly replacing explicit str-indexset.names with indexset_i.name), then it's good to go from my side :)

# teardown() ...
```
Profiler output will be written to '.profiles/{testname}.prof'
"""
Copy link
Member

Choose a reason for hiding this comment

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

This looks very handy, but I'm somewhat surprised that pytest doesn't offer something like this itself (I checked and couldn't find anything quickly, at least).

tests/data/test_optimization_indexset.py Outdated Show resolved Hide resolved
tests/data/test_optimization_indexset.py Outdated Show resolved Hide resolved
tests/data/test_optimization_indexset.py Outdated Show resolved Hide resolved
tests/data/test_optimization_indexset.py Outdated Show resolved Hide resolved
tests/data/test_optimization_indexset.py Outdated Show resolved Hide resolved
tests/data/test_optimization_indexset.py Outdated Show resolved Hide resolved
tests/data/test_optimization_table.py Outdated Show resolved Hide resolved
tests/data/test_optimization_table.py Outdated Show resolved Hide resolved
tests/data/test_optimization_table.py Outdated Show resolved Hide resolved
meksor and others added 8 commits August 13, 2024 10:20
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
@meksor
Copy link
Contributor Author

meksor commented Aug 13, 2024

Thanks for the last consistency comments, this is gtg from my side now too.

Co-authored-by: Fridolin Glatter <83776373+glatterf42@users.noreply.github.com>
@meksor meksor merged commit 227fbcb into main Aug 13, 2024
10 checks passed
@meksor meksor deleted the enhancement/simpler-test-data branch August 13, 2024 08:42
glatterf42 added a commit that referenced this pull request Aug 22, 2024
glatterf42 added a commit that referenced this pull request Aug 22, 2024
* Remove Never type hints from core; mypy complained in test file
glatterf42 added a commit that referenced this pull request Aug 22, 2024
glatterf42 added a commit that referenced this pull request Aug 22, 2024
glatterf42 added a commit that referenced this pull request Sep 30, 2024
glatterf42 added a commit that referenced this pull request Sep 30, 2024
* Remove Never type hints from core; mypy complained in test file
glatterf42 added a commit that referenced this pull request Sep 30, 2024
glatterf42 added a commit that referenced this pull request Oct 3, 2024
* Include optimization parameter basis (#79)
* Fix references to DB filters in docs
* Streamline naming in tests
* Fix and test parameter list and tabulate for specific runs
* Make indexset-creation a test utility
* Incorporate changes from #110
* Use pandas for updated add_data behaviour
* Raise minimum pandas version to enable add_data upsert
* Generalize UsageError for more optimization items
* Use generalized UsageError for Table
* Use own errors for Parameter
glatterf42 added a commit that referenced this pull request Oct 3, 2024
* Remove Never type hints from core; mypy complained in test file
glatterf42 added a commit that referenced this pull request Oct 3, 2024
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