Getter/setter idiom in Trac (or Python, for that matter)

70 views
Skip to first unread message

Christopher Nelson

unread,
Aug 15, 2012, 11:43:40 AM8/15/12
to trac...@googlegroups.com
(I've been wondering why I don't get answers on trac-dev and dug into
my Google Groups settings. After merging with another company last
year, our e-mail has been slowly migrating to a different domain. I
can -- with effort -- send as if from the old domain but the right
thing to do would see to be to change my Google Group membership to be
from the new e-mail address. I can managed my membership but only 3
of the 5 e-mail addresses associated with my Google account show up.
I have no idea why. And, of course, Groups has no help that I can
find. So I've changed my membership to be from GMail, not
work.<sigh>)

The TracPM part of my TracJSGantt plugin attempts to hide details of
other plugins and the TracPM internals from users of a plugin-agnostic
PM API. For example, the Gantt chart gets the start of a task with:

self.pm.start(ticket)

which invokes a TracPM method defined as:

# Return computed start for ticket as a Python datetime object
def start(self, ticket):
...


but I'm now at a point where I need to *set* the start date (if only
in other parts of the plugin) and I'd like to wrap that setting in a
method to ease maintenance. But I haven't done that before and I'm
unsure of what idiom is common for getter/setter method pairs. Can
anyone here provide some guidance? Thanks.

Chris
--
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Norman Harman

unread,
Aug 15, 2012, 11:53:41 AM8/15/12
to trac...@googlegroups.com
Python idiom is to not write getter / setters until (if) you need
them. Use attributes instead. Python Descriptors enable this.
http://docs.python.org/howto/descriptor.html

class Foo():
def __init__(self):
self.something = None

users of Foo can get / set .something as they wish. Later on it's
decided that setting something needs some logic. Foo is updated

def __init__(self):
self._something = None

def set_something(self, val):
# do stuff
self._something = val
def get_something(self):
return self._something # or calculate it or whatever

something = property(get_something, set_something)

Olemis Lang

unread,
Aug 15, 2012, 12:19:05 PM8/15/12
to trac...@googlegroups.com
you can also do this in case you don't want get_something and
set_something methods in that class (<= which are not as Pythonic as
uniform attribute access via properties and descriptors in general ;)

{{{
#!python

class Foo():
def __init__(self):
self.something = None

@property
def something(self, val):
# do stuff
self._something = val

@something.setter
def something(self):
return self._something # or calculate it or whatever

}}}

--
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Christopher Nelson

unread,
Aug 15, 2012, 1:34:18 PM8/15/12
to trac...@googlegroups.com
Thanks. That's fascinating and somewhat helpful but I'm not sure how
to use it in my case. My object model is kind of messy. I have, very
roughly, something like:

class TracPM:
def start(self, task):
...do something to get start out of task...
def finish(self, task):
...do something to get finish out of task...

which is then called like:

for t in tasks:
start = pm.start(t)

If I create TracPM.__get__() that doesn't relate to the start date, it
relates to the TracPM class. I guess what I need to do is:

* Create a ScheduleDate (or PMDate or something) class
* Give it __get__ and __set__ (and __delete__?) methods
* Make the dates in the task instances of that class
* Where I currently do "s = pm.start(t)" I'd do "s = t.start" (which
would invoke the getter) and elsewhere I could do "t.start =
<somedate>" to set it.

Does that sound right?


Chris

Christopher Nelson

unread,
Aug 15, 2012, 1:59:19 PM8/15/12
to trac...@googlegroups.com
On Wed, Aug 15, 2012 at 12:19 PM, Olemis Lang <ole...@gmail.com> wrote:
> you can also do this in case you don't want get_something and
> set_something methods in that class (<= which are not as Pythonic as
> uniform attribute access via properties and descriptors in general ;)
>
> {{{
> #!python
>
> class Foo():
> def __init__(self):
> self.something = None

I take it from the code that follows that you mean "self._something = None"?

>
> @property

That "@property" tells Python (or Trac?) something about how the
following definition is used?

> def something(self, val):
> # do stuff
> self._something = val
>
> @something.setter

Same question as about "@property"

> def something(self):
> return self._something # or calculate it or whatever

How is that class used?

Ethan Jucovy

unread,
Aug 15, 2012, 1:59:57 PM8/15/12
to trac...@googlegroups.com

Not quite -- you don't define __get__ and __set__ methods.  You define methods with any name, and then magically turn them into a property (which is a pair of setter/getter methods that magically act like an attribute instead) using the built-in property() function (or the alternate decorator-based spelling Olemis pointed out)

The problem in your case is that your getter is a function which takes one argument -- because, as you say, it's actually a getter of a property on the task, not on the TracPM class.  That means that you can't define the property on the TracPM class.  Instead you would define the getter and setter methods on whatever class "task" is an instance of, and then access it like `foo = self.task.start; self.task.start = datetime(...)`.

But if "task" is a Trac ticket, or some other class whose definition you don't control, then you can't really do that either.  At that point I guess the "most elegant" way to deal with the problem would be by wrapping the ticket in another class that adds the "start" and "finish" properties with their getters and setters:

class TracPMTask(object):
  def __init__(self, task):
    self.task = task
  def _get_start(self):
    return however_you_do_this(self.task)
  def _set_start(self, start_datetime):
    however_you_do_that(self.task, start_datetime)
  start = property(_get_start, _set_start)

...which you would use like:

for t in tasks:
  start = TracPMTask(t).start
  TracPMTask(t).new_start = datetime.datetime.now()

...but, as you can see, this is now a pretty big departure from how you're already doing things, and also begins to obfuscate what's going on more than it helps.  So really, in your case, the "most Pythonic" thing to do is probably to just keep it simple, and define a TracPM.set_start(self, task, new_start) method -- then you don't have to change any of your code, and it's totally clear to a reader what's going on.  It's perfectly idiomatic Python as long as you don't call the method setStart :-)

Christopher Nelson

unread,
Aug 15, 2012, 2:24:45 PM8/15/12
to trac...@googlegroups.com
On Wed, Aug 15, 2012 at 1:59 PM, Ethan Jucovy <ethan....@gmail.com> wrote:
> On Wed, Aug 15, 2012 at 1:34 PM, Christopher Nelson
> <chris.ne...@gmail.com> wrote:
>> ... My object model is kind of messy. I have, very
Right. But I could create a pmDate class that had getter and setter
methods and pass that around.

> But if "task" is a Trac ticket, or some other class whose definition you
> don't control, then you can't really do that either.

A task is a hash of ticket attributes, not quite a whole ticket.
Right now, my TracPM code adds a '_calc_start' entry to the dictionary
so `TracPM.start(t)` really just masks the name of the index with
something not a lot more complicated than `return t['_calc_start']`.

My thought is that if TracPM did something like:

t['_start'] = pmDate('start')
t['_finish'] = pmDate('finish')

then the caller could do

for t in tasks:
start = t['_start'] # Invoke the getter

and my internal code could do:

t['_start'] = somedate # Invoke the setter

Am I still off base?


> At that point I guess
> the "most elegant" way to deal with the problem would be by wrapping the
> ticket in another class that adds the "start" and "finish" properties with
> their getters and setters:
>
> class TracPMTask(object):
> def __init__(self, task):
> self.task = task
> def _get_start(self):
> return however_you_do_this(self.task)
> def _set_start(self, start_datetime):
> however_you_do_that(self.task, start_datetime)
> start = property(_get_start, _set_start)
>
> ...which you would use like:
>
> for t in tasks:
> start = TracPMTask(t).start
> TracPMTask(t).new_start = datetime.datetime.now()
>
> ...but, as you can see, this is now a pretty big departure from how you're
> already doing things,

I'm willing to write off a false start to start doing it right. There
are only a handful of uses in a couple of classes so fixing it now is
easy.

> and also begins to obfuscate what's going on more than
> it helps. So really, in your case, the "most Pythonic" thing to do is
> probably to just keep it simple, and define a TracPM.set_start(self, task,
> new_start) method -- then you don't have to change any of your code, and
> it's totally clear to a reader what's going on. It's perfectly idiomatic
> Python as long as you don't call the method setStart :-)

Gawd, I had embedded underscores!

Ethan Jucovy

unread,
Aug 15, 2012, 5:09:52 PM8/15/12
to trac...@googlegroups.com
On Wed, Aug 15, 2012 at 2:24 PM, Christopher Nelson <chris.ne...@gmail.com> wrote:
Right.  But I could create a pmDate class that had getter and setter
methods and pass that around.

You would want to create a pmTask class, not a pmDate class.  The getter and setter will manipulate a piece of data on the class that has the getter and setter: they'll be a getter and setter for the "date" aspect of the "task" object.
 
The key to understanding this is that the getter and setter magic are really just syntactic sugar for certain kinds of methods on class instances.  Once you have a class with .get_foo() and .set_foo(val) methods, you can use the syntactic sugar to reduce calls to those methods to what looks like attribute access and assignment.

A task is a hash of ticket attributes, not quite a whole ticket.

Okay, that seems feasible then -- presumably you control the code where these "task" objects are instantiated.  You can't put properties on a hash or any other built-in data type, so you'll need to create a full class for your "task" objects.  When you instantiate these "task" objects, you'll just create instances of your pmTask class and pass in the necessary hash of ticket data.  The class definition will then contain the syntactic sugar for accessing and mutating the magic "start" and "finish" properties.

Right now, my TracPM code adds a '_calc_start' entry to the dictionary
so `TracPM.start(t)` really just masks the name of the index with
something not a lot more complicated than `return t['_calc_start']`.

So I think you'd want a class definition like

{{{
class pmTask(object):
  def __init__(self, ticket_data):
    self.ticket_data = ticket_data
  def _get_start(self):
    return self.ticket_data['_calc_start']
  def _set_start(self, start):
     self.ticket_data['_calc_start'] = start
  start = property(_get_start, _set_start)
  def _get_finish(self):
    [etc]
}}}

Just to spell out fully what's happening here, this is all straightforward non-magical code until you get to the ``start = property(_get_start, _set_start) line`` -- which defines an attribute ``pmTask.start`` that secretly calls pmTask._set_start() when assignment occurs and pmTask._get_start() when access occurs.

TracPM would then set start and finish by invoking the setter like so (assuming "t" is now an instance of pmTask)

{{{
t.start = somedate
t.finish = someotherdate
}}}

Note that this is completely equivalent to ``t._set_start(somedate)`` which itself just reduces to ``t.ticket_data['_calc_start'] = somedate``.

And caller code would invoke the getter like:

{{{
for t in tasks:
  start = t.start  # under the hood Python is just treating this like ``start = t._get_start()``
}}}

The downside of defining the pmTask class in this way is that every *other* piece of ticket data now has to be accessed via the t.ticket_data dict -- t.ticket_data['id'] instead of just t['id'] and so on.  To clean this up, it's actually possible for your pmTask class to just be a subclass of the built-in dict type instead:

class pmTask(dict):
  def _get_start(self):
    return self['_calc_start'] # ``self`` is now dict-like in all respects, so we can just use the magic key in the ``self`` dict
  def _set_start(self, start):
     self['_calc_start'] = start
  start = property(_get_start, _set_start)

You'd then instantiate the objects by passing in a dict, and you'd be able to treat them just like dicts in all circumstances but with the added bonus of having this magic ``.start`` property that hides the details of how "start" is generated.

Hope this helps,
Ethan
Reply all
Reply to author
Forward
0 new messages