Bugfix for Germ Selection Line Labels and Instrument Specifier Type Error Bug #508
Conversation
| @@ -405,8 +405,8 @@ def instrument_specifier(self, name): | |||
| ------- | |||
| str or dict | |||
| """ | |||
| if name in self.nonstd_instruments: | |||
| return self.nonstd_instruments[name] | |||
| if tuple(name) in self.nonstd_instruments.keys(): | |||
There was a problem hiding this comment.
Is the convention here that the keys of nonstd_instruments are always tuples? If other hashable types are allowed this should be written so as to accept those options too. One way to do so would be to use the get method for dictionaries.
There was a problem hiding this comment.
Actually, I can answer this. I took a look at the unit test that was failing on this branch and it confirms that there was and intention for these to also have the option for string valued keys. See here.
We could rewrite this to account for both options at the level of this method, but the thing is it should be necessary to cast the name to a tuple here. If you're fixing this because you ran into a problem here it suggests to me that something upstream of this method is using malformed inputs, or has inadvertently hardcoded in the expectation that this is always a string (and not a tuple). My preference would be to track this down, but if folks would rather patch this to accept both strings and tuples (casting if needed) to get this working I won't push back too hard.
| if ckt._static: | ||
| new_ckt = ckt.copy(editable=True) | ||
| new_ckt.line_labels = target_model.state_space.state_space_labels | ||
| print(new_ckt.line_labels) |
There was a problem hiding this comment.
Nix this print statement.
Bugfix for #507 and instrument specifier type error bug.