Skip to content

Conversation

@eordog
Copy link
Contributor

@eordog eordog commented Nov 7, 2025

No description provided.

@JoelPasvolsky JoelPasvolsky self-requested a review November 7, 2025 22:27
Copy link
Collaborator

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eordog, the doctest error on ValueError: array must not contain infs or NaNs is tricky but I can get it fixed locally (it requires NumPy > 2.6.2, for example 2.3.4 to recreate) with the changes in this review. I suspect that what is happening is that the test setups are getting mixed when run together before the code tests. I've reduced the setups to just two, one before the NL code examples and one for the CQM example, and separated these into two groups.

Locally when I run now I get just the expected NumPy errors on True_.

If this looks right to you, we'll see if a new commit runs in CI also without the ValueError: array must not contain infs or NaNs error, and then the remaining 8 NumPy errors will be fixed by @randomir's PR.

I'll review the rest separtely.

Copy link
Collaborator

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

Copy link
Collaborator

@JoelPasvolsky JoelPasvolsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

(Just suggest adding the two linespaces to the code I mentioned)

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.

3 participants