AbstractGPs.jl icon indicating copy to clipboard operation
AbstractGPs.jl copied to clipboard

Move AbstractGPs related test from GPLikelihoods here

Open theogf opened this issue 3 years ago • 9 comments

I removed the tests using AbstactGPs from GPLikelihoods and saved them in this gist. They should probably be added here (or ApproximateGPs) to check compatibility.

theogf avatar Dec 20 '21 16:12 theogf

I think I would prefer ApproximateGPs, but just because we already have this as a dependency there. I could be convinced otherwise.

willtebbutt avatar Dec 20 '21 16:12 willtebbutt

I really don't mind either, but somehow I think these tests could still fit GPLikelihoods just not in the interface module.

theogf avatar Dec 20 '21 16:12 theogf

Sorry, I'm not sure what you mean. Could you elaborate?

willtebbutt avatar Dec 20 '21 16:12 willtebbutt

Well the whole point of GPLikelihoods is to work together with the rest of the ecosystem. So I think we should know when changes in GPLikelihoods might be incompatible with AppGP or AbsGP.

theogf avatar Dec 20 '21 17:12 theogf

Fair point. Taking another look at the code, it looks like it would be fine in this package (I was worried we'd need some of the code in ApproximateGPs in order to run decent tests, but the ones that we currently have seem fine without it).

Where were you thinking to locate these tests? test or src?

willtebbutt avatar Dec 23 '21 15:12 willtebbutt

Definitely test right now GPLikelihoods is not a dependency and that would not make sense to add it.

theogf avatar Dec 23 '21 15:12 theogf

Cool. I'm very happy to review if you open a PR.

willtebbutt avatar Dec 23 '21 15:12 willtebbutt

What's the reason for not just having this within the GPLikelihoods package ?

st-- avatar Apr 19 '22 16:04 st--

You mean in the tests of GPLikelihoods?

theogf avatar Apr 19 '22 16:04 theogf