Python Module Exposure

[Jacob Page]

George Sakkis wrote:
> 1. As already noted, ISet is not really descriptive of what the class
> does. How about RangeSet ? It's not that long and I find it pretty
> descriptive. In this case, it would be a good idea to change Interval
> to Range to make the association easier.

The reason I decided not to use the term Range was because that could be 
confused with the range function, which produces a discrete set of 
integers.  Interval isn't a term used in the standard/built-in library, 
AFAIK, so I may stick with it.  Is this sound reasoning?

> 2. The module's "helper functions" -- which are usually called factory
> functions/methods because they are essentially alternative constructors
> of ISets -- would perhaps better be defined as classmethods of ISet;
> that's a common way to define instance factories in python. Except for
> 'eq' and 'ne', the rest define an ISet of a single Interval, so they
> would rather be classmethods of Interval. Also the function names could
> take some improvement; at the very least they should not be 2-letter
> abbreviations. Finally I would omit 'eq', an "interval" of a single
> value; single values can be given in ISet's constructor anyway. Here's
> a different layout:

First, as far as having the factory functions create Interval instances 
(Range instances in your examples), I was hoping to avoid Intervals 
being publically "exposed."  I think it's cleaner if the end-programmer 
only has to deal with one object interface.

I like the idea of lengthening the factory function names, but I'm not 
sure that the factory methods would be better as class methods. 
Consider a use-case:

 >>> import iset
 >>> interval = iset.ISet.lowerThan(4)

as opposed to:

 >>> import iset
 >>> interval = iset.lowerThan(4)

This is less typing and probably eliminates some run-time overhead.  Can 
you list any advantages of making the factory functions class methods?

> class Range(object):     # Interval
> 
>     @classmethod
>     def lowerThan(cls, value, closed=False):
>         # lt, for closed==False
>         # le, for closed==True
>         return cls(None, False, value, closed)
> 
>     @classmethod
>     def greaterThan(cls, value, closed=False):
>         # gt, for closed==False
>         # ge, for closed==True
>         return cls(value, closed, None, False)
> 
>     @classmethod
>     def between(cls, min, max, closed=False):
>         # exInterval, for closed==False
>         # incInterval, for closed==True
>         return cls(min, closed, max, closed)

I like the function names, but in my dialect, lessThan would be more proper.

> class RangeSet(object):  # ISet
> 
>     @classmethod
>     def allExcept(cls, value): # ne
>         return cls(Range.lowerThan(value), Range.greaterThan(value))
> 
> 3. Having more than one names for the same thing is good to be avoided
> in general. So keep either "none" or "empty" (preferably "empty" to
> avoid confusion with None) and remove ISet.__add__ since it is synonym
> to ISet.__or__.

I agree that "none" should be removed.  However, some programmers may 
prefer to use +, some |, so I'd like to provide both.

> 4. Intervals shouldn't be comparable; the way __cmp__ works is
> arbitrary and not obvious.

I agree that __cmp__ is being used arbitrarily.  I wanted sorting to be 
easy, but there's other ways of doing that.  I'll take your suggestion.

> 5. Interval.adjacentTo() could take an optional argument to control
> whether an endpoint is allowed to be in both ranges or not (e.g.
> whether (1,3], [3, 7] are adjacent or not).

Interval objects weren't meant to be public; adjacentTo is used by ISet 
for combinatory functions.  I could look into ways to hide the class 
from the public, but hiding things never seemed Pythonic to me; someone 
may want to use them for some reason.  Maybe pydoc can be made to not 
document certain objects.

> 6. Possible ideas in your TODO list:
>     - Implement the whole interface of sets for ISet's so that a client
> can use either or them transparently.

That was my intention.  I'll have to see which interface functions I'm 
missing.

>     - Add an ISet.remove() for removing elements, Intervals, ISets as
> complementary to ISet.append().
>     - More generally, think about mutable vs immutable Intervals and
> ISets. The sets module in the standard library will give you a good
> idea of  the relevant design and implementation issues.

Both good ideas.

> After I look into the module's internals, I'll try to make some changes
> and send it back to you for feedback.

Thanks for your feedback, George.  I look forward to any additional 
comments you come up with.