Ok.  What I'm concerned about is user code that gets written that changes
model, and later we want to change how that's implement but are bound by
how we've done it here.  I think if users don't need to access it (but
other classes in this package do), it should be a private variable.


On Wed, Mar 13, 2013 at 11:59 AM, Nadia Dencheva
<notifications@github.com>wrote:

> In astropy/models/constraints.py:
>
> > +               return xcen
> > +
> > +            tied ={'xcen':tie_center}
> > +        bounds: dict
> > +            keys: parameter names
> > +            values:  list of length 2 giving the desired range for hte parameter
> > +        eqcons: list
> > +            A list of functions of length n such that
> > +            eqcons[j](x0,*args) == 0.0 in a successfully optimized
> > +            problem.
> > +        ineqcons: list
> > +            A list of functions of length n such that
> > +            ieqcons[j](x0,*args) >= 0.0 in a successfully optimized
> > +            problem.
> > +        """
> > +        self.model = model
>
> @mdboom <https://github.com/mdboom> The guide says "Classes should either
> use direct variable access, or python’s property mechanism ...". Am I wrong
> in thinking that a property makes sense mostly when something more than
> just returning or setting the variable needs to be done? The guide for me
> in this code was to always set model parameters with properties since they
> are the variables users will interact with. Constraints and fitters keep an
> instamce of the model as an attribute because they need to manipulate its
> parameters but users don't need to access constraints.model or
> fitter.model. I don't see a benefit in making 'model' a property. A private
> variable - may be but this seems a bit arbitrary. Does this make sense?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/astropy/astropy/pull/493/files#r3356651>
> .
>



-- 
Michael Droettboom
http://www.droettboom.com/