Many thanks for tidying up some code that I was not really qualified to write, but was needed by a major user.

I think it is a well thought out design with very clear test cases (which are really the most important, these sort of second order refactors are easy when test suites guarantee correctness :) )

I have a reservation and a question

• Changing the user call-back function from void (*user_callback) to HighsCallbackFunctionType, where HighsCallbackFunctionType is

std::function<void(int, const std::string&, const HighsCallbackDataOut*, HighsCallbackDataIn*, void*)>)

breaks the API that's been defined in 1.6.0 - surely.

Not as badly as it might, since for the C-API compatibility there's also HighsCCallbackType c_callback = nullptr;, where HighsCCallbackType is the older C-compatible void* type. This can be used with Highs::setCallback (because of function overloads), as is done for the C API.

I've already announced the new call-back definition at a workshop, and at least two major users are writing callbacks on this basis. I guess that we can communicate with them - they are "friends" - rather than postpone your modifications to the release of HiGHS 2.0. I'm inclined to do this, as the advance that you've made - in cleaning-up the call-backs code and extending call-backs to the Python API - is worth the trouble.

That would be great. In terms of compatibility, if users are not able / willing to make the switch to the new type, they can replace user_callback with c_callback and if they are only using function pointers, things should be OK. In fact, as long as they use the API (Highs::setCallback) instead of populating the HighsCallback struct by hand (i.e. manually setting user_callback), everything should be backwards compatible.

All things considered equal, users of the C++ API will likely prefer to use user_callback with the new HighsCallbackFunctionType simply because it allows for more callable objects (e.g. lambdas, both with and without capture; or operator() objects). The lambda functionality is especially important for efficient Pybind11 (and is a requirement for highspy / scipy users).

• It looks to me as if the use of the unwieldy std::function<void(int, const std::string&, const HighsCallbackDataOut*, HighsCallbackDataIn*, void*)> in check/TestCallbacks.cpp can be replaced by HighsCallbackFunctionType

Yup, good catch, updated.

This PR is almost ready to go from the highspy end as well, will take it out of draft ASAP.

Many thanks for tidying up some code that I was not really qualified to write, but was needed by a major user.

I think it is a well thought out design with very clear test cases (which are really the most important, these sort of second order refactors are easy when test suites guarantee correctness :) )

Thanks: that's a level I understand, and at which I feel I operate reasonably well. It's just the detailed knowledge of C/C++ that I don't have , since I learned it "on the job" after jumping from Fortran 77 at the age of 53!

Since the SciPy callbacks require some more discussion (here and at SciPy) I think this should be ready to go in as is, @jajhall, it improves (cleans up) the implementation in HiGHs and enables callbacks for highspy.