-
Notifications
You must be signed in to change notification settings - Fork 42
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
Reorganize simulation lookup #265
Conversation
f90f817
to
d7c57f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Scienfitz, just wanted to let you know that I've rebase the PR on top of #266, which finally cleans up our optional imports and also solves the problem that remained in this PR. So let's review the other PR first 😃
d227c24
to
24c282b
Compare
24c282b
to
cfe57d8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like most of my comments were in fact not relevant since they dealt with old code, hence approve
b385912
to
6fe6e50
Compare
6fe6e50
to
cba049c
Compare
cba049c
to
379f790
Compare
379f790
to
da8ce9e
Compare
This PR is another step toward a cleaned simulation package:
Campaign
so that it can be flexibly used without having to set up a campaign object first (e.g. in tests)Moves simulation-related utilities to the simulation package