Improve doc testing #55
Conversation
arcondello
force-pushed
the
docs/flat-symbols
branch
10 times, most recently
from
04c182d to
65a6287
Compare
arcondello
added
bug
| @@ -85,31 +77,15 @@ | |||
|
|
|||
There was a problem hiding this comment.
Line 51:
language = 'en'
Let's start getting rid of WARNING: Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).
| @@ -164,8 +164,7 @@ for symbols of a model without using labels. | |||
| >>> with model.lock(): | |||
| ... for symbol in model.iter_decisions(): | |||
| ... symbol.set_state(0, [2]) | |||
| ... model.objective.state(0) == -4 | |||
| True | |||
| ... assert model.objective.state(0) == -4 | |||
There was a problem hiding this comment.
I'm not sure about this: it save one line but at the cost of slightly less understandable code for less experienced users and the replacement of a False output were it to not obtain by an error state, which is not desirable
There was a problem hiding this comment.
The problem is that NumPy either returns True or np.True_ depending on the version. So doctests may fail in either case. I believe assert is fairly clear even to novice Python users.
There was a problem hiding this comment.
Or could print the value (print(model.objective.state(0)))
There was a problem hiding this comment.
Is that really clearer than assert? Both are built-in Python operations. If we cannot assume that they know about assert, then can we assume they know print?
There was a problem hiding this comment.
Yes, they all know print
There was a problem hiding this comment.
Originally we moved from print() statements to using equals to get a True because we worried about changes to the format of the output over time (mostly unordered lists and sets) but here that's not a concern
There was a problem hiding this comment.
I am comfortable assuming that they know assert as well.
There was a problem hiding this comment.
changes to the format of the output over time (mostly unordered lists and sets) but here that's not a concern
Is it not? NumPy could decide to change the str() representation of np.True_.
There was a problem hiding this comment.
Sorry, I meant print the value: print(model.objective.state(0)) and showing its output (that will be used by the doctest)
There was a problem hiding this comment.
That will return np.float64(4.0) or 4 depending on the NumPy version
Match the SDK's doc theme and add doctests to CI.
Closes #54
Also fixes some doc errors introduced in #42 and #53