Ashley Scillitoe

Results 167 comments of Ashley Scillitoe

Offline, @ojcobb asked why we don't have a separate `reset` and `reset_state` method, which is something mentioned in the notion doc. This is because the second reset method will be...

> Beyond what we've discussed around the preemptive renaming of `reset` to `reset_state` it all looks good to me! Good work on the additional docstrings too! Thanks @ojcobb !

The 3.7 build failure is occurring before we even reach the state handling functionality. It appears to be a not-seen-before issue with the `MMDDriftOnlineTF` detector itself. See https://github.com/SeldonIO/alibi-detect/issues/706.

> Perhaps a silly question about the general functionality? Is the intention that the user is saving state to the detector save directory or elsewhere? > > ```python > from...

> > Offline, @ojcobb asked why we don't have a separate `reset` and `reset_state` method, which is something mentioned in the notion doc. > > This is because the second...

> e.g. `reset` identifying an online detector that has some state - this means we can't repurpose `reset` for anything else in the future This is a really important point...

> Maybe I'm missing something but I thought `reset` didn't exist at all until this PR? Agree that using `reset_state` likely makes most sense. It did exist: ```python def reset(self)...

> Ah ok. Shall we introduce reset_state and deprecate the currently existing reset then? I've done this in [aa6b0d3](https://github.com/SeldonIO/alibi-detect/pull/604/commits/aa6b0d382dced113678f2642982f38f52e00b7f8), but kept `reset` as a link to `reset_state` with a `DeprecationWarning`....

> @ascillitoe Is this file move ok? I thought `conftest.py` should always be inside a folder named `tests`? [alibi_detect/saving/tests/conftest.py → alibi_detect/conftest.py](https://github.com/SeldonIO/alibi-detect/pull/604/files#diff-e1cda4eb9074d4d0004843812944b7e3d382617320a0d7a6c2b4315363f91a9a) Doesn't this make `conftest` a public module of `alibi-detect`...

[d190589](https://github.com/SeldonIO/alibi-detect/pull/604/commits/d1905893a31ae8b663479087295b078de5b4da82) removes `save_state` from `save_detector`. The new logic is to always save state if `self.t > 0`. If this is not desired, `.reset()` can be called prior to using `save_detector`.