Bnb/dev #108
Bnb/dev #108
Conversation
sup3r/pipeline/forward_pass.py
Outdated
| self.preflight() | ||
|
|
||
| @property | ||
| def worker_attrs(self): |
There was a problem hiding this comment.
This seems like a bad use of a property. Properties enable encapsulated compute. Whats the reason to have a hard-coded property value?
There was a problem hiding this comment.
Yeah no reason. I guess better as a method?
There was a problem hiding this comment.
No no my point was just make it an attribute! self._protected attribute if you don't want it messed with. Doesnt make sense to have any sort of dynamic code return a hard-coded value.
There was a problem hiding this comment.
Ah ok. I made it a property because its abstracted in InputMixIn and defined in both forward pass and data handling. I added an underscore :)
There was a problem hiding this comment.
Lol why is it a property anywhere though??? Why wouldn't we just set self._worker_attrs = [...] in all of the init's? Hardcoded properties don't make sense - abstracted upstream or not.
Misc cleaning. Discussed changes to qa utilities.