More type annotations #104
Conversation
…uff) of class FactBase
clorm/clingo.py
Outdated
| from clingo import ( core, Logger, MessageCode, TruthValue, version, symbol, Function, Infimum, | ||
| Number, String, Supremum, Symbol, SymbolType, Tuple_, parse_term, | ||
| symbolic_atoms, SymbolicAtom, SymbolicAtoms, theory_atoms, TheoryAtom, | ||
| TheoryElement, TheoryTerm, TheoryTermType, util, solving, ModelType, | ||
| SolveControl, SolveResult, propagator, Assignment, PropagateControl, | ||
| PropagateInit, Propagator, PropagatorCheckMode, Trail, backend, Backend, | ||
| HeuristicType, Observer, configuration, Configuration, statistics, | ||
| StatisticsArray, StatisticsMap, StatisticsValue, control, | ||
| application, Application, ApplicationOptions, Flag, clingo_main) | ||
|
|
There was a problem hiding this comment.
Explicitly importing individual clingo attributes was something I was trying to avoid because it adds a burden to maintain the list as clingo changes. Also if there is some variation between different clingo versions then you have to take this into account. Currently, provided the important classes haven't changed (Control, Model, etc) then it doesn't matter what other things change with clingo.
There was a problem hiding this comment.
Yeah you might be right. This was just my first working version to make mypy happy. Of course these changes are just usefull if you use mypy in your project and don't add any extra value to clorm.
Another solution perhaps would be to move all the Override-Classes into a separate file (lets call it clorm._clingo.py) and in clorm.clingo.py we write
if TYPE_CHECKING:
from ._clingo import *
from clingo import * # would be a mypy error because SolveHandle, Model and Control already exists
else:
# change the order so python will use the overriden/wrapped classes
from clingo import *
from ._clingo import *During typechecking mypy just sees the classes imported from clorm._clingo.py. At runtime we have to change the order because python always uses the last definition of an object (with the same name). But it looks also a bit hacky...
A further alternative would be to rename Control, Model and SolveHandle to avoid the name clashes but this introduces quite some (breaking) changes. And i don't know if it's worth doing this.
Perhaps i should revert these changes (as it has actually nothing todo with type annotations...), find a proper solution and handle it in a separate PR or just leave it to the user to deal with the original mypy error.
What do you think?
There was a problem hiding this comment.
I'd be in favour of reverting this part and looking at it as a separate PR. My gut feeling is that it should be possible to do in a clean-ish way.
For example, I was wondering if it would be possible to construct this list of clingo imports by using the dir() function?. The following is probably not valid but I maybe something like it:
clingo_imports = dir(clingo) - <things we don't want>
from clingo import *clingo_imports
There was a problem hiding this comment.
I'm not sure, but i don't think that something similar will work as mypy is a static type checker and doesn't execute any code.
This PR adds type annotations to most of the 'public' functions and methods (see #62)
It also fixes some mypy issues when unpacking a fact or use clorm's Control, Model or SolveHandle classes.
The query-API is not annotated with types. Theoretically it would be possible to add type annotations by using
TypeVarTuple,Genericand some clever tricks to infer also the correct type when using joins, but there is a technical limitation which makes this not possible to use with py36 (see here)