search

dssg / triage

General Purpose Risk Modeling and Prediction Toolkit for Policy and Social Good Problems
Other
185 stars 60 forks source link

Closes #643 #788

Closed nanounanue closed 4 years ago

nanounanue commented 4 years ago

Closes #643

shaycrk commented 4 years ago

Definitely a better default behavior and makes sense to me generally. Only question is whether it might be a little cleaner structurally to have a "no-op" importance method rather than a separate class?

nanounanue commented 4 years ago

Totally agree, I don' t like it either, but that's how triage does it :( . (See LabelGeneratorNoOp, EntityDateTableGeneratorNoOp, ProtectedGroupsGeneratorNoOp, etc)

I need to change the structure (and incorporate the class instead of many args, etc). I will do a new issue for all of this

shaycrk commented 4 years ago

Ah, didn't realize that pattern was used elsewhere -- makes sense to be consistent in that case. I'll go ahead and merge in that case!

thcrock commented 4 years ago

The null object pattern used here is so the interface between components can stay the same regardless. If you make a no-op method on each instead, it means you have to make if statements buried in loops in different places. Using null objects like these, the if statements can be in one place: when you build the components based on config.