Nicholas Bergson-Shilcock wrote:
Excerpts from Rob Miller's message of Fri Jan 23 13:30:36 -0500 2009:
Nicholas Bergson-Shilcock wrote:
While troubleshooting a bug with the recently added next/previous thread links,
I noticed that getToplevelMessages[1] was not behaving as I expected it to.
Specifically, there were two places where I wanted different behavior:
1. The function takes an optional boolean argument, recent_first. When true,
the method sorts the threads by their modification dates, with the thread with
the most recent response listed first. Unexpectedly (at least to me), when
recent_first is false (the default), the threads are returned sorted by their
dates, i.e., by when the initial message was sent. Thus this is *not*
necessarily the reverse ordering from when specifying recent_first = True.
2. getToplevelMessages also takes optional start and end arguments for
specifying a date range. Again, this range uses the dates and not the
modification dates of the messages. This leads to confusion, particularly when
sorting on the modification, since you will likely not get the results you
expect (e.g., you'll likely get messages that you thought should have been
filtered out).
I'd like to modify the function to both sort and filter by modification_date.
This seems to me to be more sensible behavior, particularly because it provides
the expected behavior when dealing with threads. I've grep'd through the code
and it doesn't look like it will break anything, but I wanted to shoot an
e-mail out to the list to make sure no one was relying on the current behavior
or had some reason to prefer it.
i agree that we should consider some changes here, but i'm not completely
clear on what you're proposing. for instance, for #1 above, i would not
expect recent_first being False to necessarily reverse the order in which the
threads are returned, so if that's what you're thinking it would do i'm not
sure i like that idea.
Yes, that is what I was thinking. I agree that setting recent_first to False
does not *necessarily* (in the logical sense) need to return the reversed list,
however, I still think it's a good idea. At a minimum, the interface needs to
be more clearly defined, as right now the only way to know how things are
ordered is to read the actual code. Is there a present use case for doing
otherwise?
maybe what we need is a better API altogether. instead of a single boolean,
we might accept a 'sort_key' argument and an optional 'descending' or
'reversed' boolean argument. this would allow for more expressiveness and
more clarity. would doing things this way meet your needs? does it
complicate things enough to make you balk at wanting to do so right now?
I'm +0 on this, mostly because I think adding this on top of recent_first would
be clunky, and doing away with recent_first would change the established
interface. My needs could definitely be met by this API, however, assuming
there were
both sort_on and filter_on (or something similar) keywords.
i'd prefer that we not change the recent_first behavior, but instead deprecate
the use of that argument, so that if it's passed in to the method a
deprecation warning is sent (using the python 'warnings' package) to the log,
like so:
_marker = object()
def getToplevelMessages(self, start=None, end=None, recent_first=_marker,
sort_on=None, filter_on=None, descending=False):
if recent_first is not _marker:
warnings.warn("Use of the 'recent_first' argument to the "
"getToplevelMessages method is deprecated. "
"Please use the 'sort_on', 'filter_on', and "
"'descending' arguments to achieve the desired "
"results", DeprecationWarning)
# following would be code that sets 'sort_on' and 'descending'
# so as to emulate the correct behavior depending on what
# 'recent_first' was set to
as long as the default behavior (i.e. the behavior when neither recent_first
nor one of the new keyword args) is the same, this seems to me a reasonable
transition path.
-r
--
Archive:
http://www.openplans.org/projects/listen/lists/listen-dev/archive/2009/01/1232744259646
To unsubscribe send an email with subject "unsubscribe" to
[email protected]. Please contact [email protected]
for questions.
