-
Notifications
You must be signed in to change notification settings - Fork 4
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
Enforce order in which modules are dealt with in _do_on_generic_first_appt #1396
Enforce order in which modules are dealt with in _do_on_generic_first_appt #1396
Conversation
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.
This looks correct to me - you could do the filtering and ordering a bit more directly as suggested below, but I think what you have written is equivalent to this so no need to change this if the plan is just to keep this for testing and not merge.
modules: OrderedDict[str, "Module"] = fixed_modules_order #self.sim.modules | ||
filtered_modules = [item for item in modules if item in self.sim.modules] | ||
|
||
symptoms = self.sim.modules["SymptomManager"].has_what(self.target) |
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.
modules: OrderedDict[str, "Module"] = fixed_modules_order #self.sim.modules | |
filtered_modules = [item for item in modules if item in self.sim.modules] | |
symptoms = self.sim.modules["SymptomManager"].has_what(self.target) | |
filtered_and_sorted_modules = [ | |
self.sim.modules[name] for name in fixed_modules_order if name in self.sim.modules | |
] | |
symptoms = self.sim.modules["SymptomManager"].has_what(self.target) |
for name in filtered_modules: | ||
module = self.sim.modules[name] |
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.
for name in filtered_modules: | |
module = self.sim.modules[name] | |
for module in filtered_and_sorted_modules: |
Thank you @matt-graham - jobs submitted, nothing left to do but keep our fingers crossed! |
Results of this PR were included in last comment to PR #1389. Order was confirmed to have no significant effect on outputs. |
Quick PR to ensure we can specify in which order modules are dealt with in _do_on_generic_first_appt. This will have to be changed manually, and will only be used for testing (will not be merged in master).
@matt-graham could you just double check this looks all right to you?