DONTMERGE: improve repr by a lot #1276
Conversation
|
Checking updated PR...
Comment last updated on March 11, 2018 at 23:31 Hours UTC |
|
Is this work in progress? |
Yes, but the functionality is all there, I just need to go through the remaining parts of the library ( |
|
That said, you can definitely have a look at the tooling and the outcome in, e.g., the |
| posargs = [self.domain] | ||
| optargs = [('range', self.range, self.domain ** self.domain.ndim), | ||
| optargs = [('range', self.range, self.domain), |
| @@ -68,10 +70,27 @@ def __init__(self, space, exponent): | |||
| Domain of the functional. | |||
| exponent : float | |||
| Exponent for the norm (``p``). | |||
|
|
|||
|
|
|||
| -------- | ||
| >>> space = odl.rn(2) | ||
| >>> l1norm = odl.solvers.LpNorm(space, exponent=1) | ||
| >>> l1norm |
There was a problem hiding this comment.
Do we really need this extra line?
There was a problem hiding this comment.
I was guessing that already 😉
| -------- | ||
| >>> pspace = odl.ProductSpace(odl.rn(2), 2) | ||
| >>> l1_2_norm = odl.solvers.GroupL1Norm(pspace, exponent=2) | ||
| >>> l1_2_norm |
There was a problem hiding this comment.
Per above (and in the future) I think this line is redundant
|
|
||
|
|
||
| class IndicatorGroupL1UnitBall(Functional): | ||
|
|
||
| """The convex conjugate to the mixed L1--Lp norm on `ProductSpace`. | ||
| """Indicator functional of the unit ball in the Group-L1 norm. |
There was a problem hiding this comment.
For all of these, we should perhaps be more clear in what we mean by "indicator function" since this has different meanings in different contexts.
|
|
||
| This functional maps all elements in the domain to a given, constant value. | ||
| """ | ||
| """Functional mapping all inputs to the same constant.""" |
There was a problem hiding this comment.
perhaps add the word "scalar" before constant
| Examples | ||
| -------- | ||
| >>> space = odl.rn(2) | ||
| >>> op = odl.solvers.ConstantFunctional(space, 1.5) |
There was a problem hiding this comment.
Just squeeze these to one line, e.g.
>>> odl.solvers.ConstantFunctional(odl.rn(2), 1.5)
ConstantFunctional(rn(2), constant=1.5)| >>> space = odl.rn(3) | ||
| >>> func = odl.solvers.IndicatorBox(space, 0, 2) | ||
| >>> func | ||
| IndicatorBox(rn(3), lower=0, upper=2) |
There was a problem hiding this comment.
Nice use of named arguments per default, this is much easier to read!
| """ | ||
| posargs = [self.domain] | ||
| with npy_printoptions(precision=REPR_PRECISION): | ||
| inner_parts = signature_string_parts(posargs, []) |
There was a problem hiding this comment.
Short thought here, shouldn't the "[]" be a default?
There was a problem hiding this comment.
I guess I could make that the default.
| allow_mixed_seps=False) | ||
|
|
||
|
|
||
| #TODO: continue here |
|
Any chance to get this done, improved reprs is always welcome :) |
|
I'm working my way through it, slowly. Still stuff left to do. Will look into it this week. |
- add helper `_separators` to determine string separators that keep a `repr` string within a given line width - split repr string rendering into two functions, one to determine the parts to be printed and one to compile the final string (with separators) - `attribute_repr_string` helper to class attributes - `element_repr_string` helper to print space elements - make repr printing of product space elements more consistent - improve default repr of `Operator` (now actually good by default, but unspecific) - use Numpy's print options as a single point to configure printing - add doctests to lots of `__repr__` methods Further miscellaneous changes: - fix bug of `uniform_discr_fromdiscr` ignoring dtype - sort imports using the `isort` tool (mode `-m 4`) - add `asweighted` to base_tensors to make weighted and unweighted spaces compatible in tensor operators - remove base classes for operator and adjoint in favor of inner adjoint classes (less code) - make most default operators take `domain` and `range` and implement the corresponding adjoints/inverses etc. - implement `noise_array` for product spaces that are not power spaces - remove the `almost_equal` test util - improve a number of doctests (use `odl.` calls, better names etc.) - make comparison of arrays in tests faster - implement broadcasting of shapes (M, N) and (N,) in product spaces - remove some remaining doc glitches, mostly `FnBase` TODO: - go through rest of the library - replace `almost_equal` by `pytest.approx` - tests for pspace broadcasting - make `uniform_discr_fromdiscr` respect weighting - fix failing unit tests
| """Return ``str(self)``.""" | ||
| dom_ran_str = '\n-->\n'.join([repr(self.domain), repr(self.range)]) | ||
| return '{}:\n{}'.format(self.__class__.__name__, indent(dom_ran_str)) | ||
| optmod = ['!r', '', '', ''] |
There was a problem hiding this comment.
This shouldn't be necessary
Summary:
_separatorsto determine stringseparators that keep a
reprstring within agiven line width
one to determine the parts to be printed and
one to compile the final string (with separators)
attribute_repr_stringhelper to class attributeselement_repr_stringhelper to print spaceelements
consistent
Operator(now actuallygood by default, but unspecific)
configure printing
__repr__methodsFurther miscellaneous changes:
uniform_discr_fromdiscrignoring dtypeisorttool (mode-m 4)asweightedto base_tensors to make weightedand unweighted spaces compatible in tensor operators
favor of inner adjoint classes (less code)
domainandrangeand implement the corresponding adjoints/inverses etc.
noise_arrayfor product spaces thatare not power spaces
almost_equaltest utilodl.calls,better names etc.)
in product spaces
FnBaseTODO:
almost_equalbypytest.approxuniform_discr_fromdiscrrespect weighting