Skip to content

Conversation

@Swayam-maurya
Copy link
Contributor

This PR skips the nag_dfols optimizer tests when the optional DFO-LS dependency
is not available.

Without this guard, pytest fails during test collection when DFO-LS is not
installed locally. The change makes the test suite robust to optional optimizer
dependencies, consistent with how other optimizers are handled.

Copy link
Member

@janosg janosg left a comment

Choose a reason for hiding this comment

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

Hi @Swayam-maurya, thanks a lot for the PR! yes, this is something we want. Could you change it to use @pytest.mark.skipif instead of using pytest.skip inside an if condition? This makes it more aligned with the rest of optimagic. You can find an example here:

@codecov
Copy link

codecov bot commented Jan 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Swayam-maurya
Copy link
Contributor Author

Thanks for the suggestion!..
I’ve updated the test to use pytest.mark.skipif for the optional DFO-LS dependency and verified locally that all optimizer tests pass.

Copy link
Member

@janosg janosg left a comment

Choose a reason for hiding this comment

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

Thanks! This is ready to merge. I'll investigate the failing docs build; It's unrelated to your changes!

@Swayam-maurya
Copy link
Contributor Author

Thanks a lot! Appreciate you looking into the docs build.

@janosg janosg merged commit 2d618ae into optimagic-dev:main Jan 23, 2026
26 checks passed
@Swayam-maurya Swayam-maurya deleted the first-pr-setup branch January 24, 2026 18:21
@Swayam-maurya
Copy link
Contributor Author

Hi @janosg ...Thanks again for merging the PR.
I’m looking for another small, well-scoped issue to work on.
Is there something you’d recommend ?

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