Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: corrected bug that prevented combined fits with multiple x-obs in some cases #241

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

s-kuberski
Copy link
Collaborator

It was not possible to perform combined fits which rely on more than one x-obs when the data sets had unequal numbers of observations. The case that failed is given in the newly implemented test function.
The bugfix ensures that the data sets are combined appropriately.

@s-kuberski s-kuberski requested a review from fjosw as a code owner September 13, 2024 07:31
@fjosw
Copy link
Owner

fjosw commented Sep 13, 2024

We seem to have bad luck with the rng. I fixed one of the failing tests by slightly increasing the tolerance but now also test_merge_idx is failing. There I'm not so sure what causes this 🤔

@s-kuberski
Copy link
Collaborator Author

That’s indeed weird. Everything went fine on my local machine and I would not have thought that there was randomness involved in this test. I’ll try to resolve it.

@s-kuberski
Copy link
Collaborator Author

There seemed to be a coincidence where we were (actually, it was my faulty implementation of the test) testing the equality of a range and a list in certain circumstances. Double bad luck, I would say. I've fixed the test.

@fjosw fjosw merged commit 4b1bb08 into fjosw:develop Sep 13, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants