Right now, ML_Lag(...,spat_diag=True) ignores spat_diag. For the GM_* models, they're running the AK test, which is fine. For the ML models, we might want to either:
add to the docstring that spat_diag is ignored for the ML models, since there's no relevant tests.
remove the spat_diag option from the keyword arguments.
I do like the ability to keep the API largely the same across the classes, but I also don't like having an argument that does nothing, even if documented.
ljwolf
commented
6 years ago
Right now,
ML_Lag(...,spat_diag=True)ignoresspat_diag. For theGM_*models, they're running the AK test, which is fine. For the ML models, we might want to either:spat_diagis ignored for the ML models, since there's no relevant tests.spat_diagoption from the keyword arguments.I do like the ability to keep the API largely the same across the classes, but I also don't like having an argument that does nothing, even if documented.
Thoughts from other spreg devs?