[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#523998: python-apt: Package class should have a get_version method



(Please remember to send your emails to the bug in future)

On Wed, Apr 15, 2009 at 09:52:14AM +0200, Sebastian Heinlein wrote:
> Am Dienstag, den 14.04.2009, 14:25 +0200 schrieb Julian Andres Klode:
> > On Tue, Apr 14, 2009 at 09:22:26AM +0200, Sebastian Heinlein wrote:
> > > Package: python-apt
> > > Version: 0.7.7.1+nmu1
> > > Severity: wishlist
> > > Tags: patch
> > > 
> > > The Package class should provide a nice way to get a corresponding Version 
> > > instance by specifing the version string.
> > > 
> > > E.g.: Package.get_version("242-1")
> > 
> > We could also support Package.versions["242-1"], by providing a class VersionList
> > with a special __getitem__()
> > 
> > The code listed below would allow us to do things like::
> > 
> >     '242-1' in Package.versions
> >     Package.versions['242-1']
> > 
> > While still allowing stuff like::
> > 
> >     Package.versions[0]
> 
> I would be more carefully with adding additional classes and attaching
> instances to the package instance. Since we have got 20000 Package
> instances we have to consider memory consumption. (By the way I made a
> test creating the Package instance only if it is requested and only
> storing the pkgCache::Package in the dictionary - it saved 8 MByte).
You don't need to store pkgCache::Package in dictionaries, pkgCache itself
can be accessed as a dictionary. I have a patch in my branch which uses a
set to list all available package names and create the package objects on
the fly. This saves 10 MB here.

And it also keeps weak references to the created package objects, so accessing
a package multiple times gives you the same object (in optimal cases). If the
second object is different, the first object has already been collected by GC.

> 
> But I haven't checked how much the extra memory a VersionList instance
> per Package would take.
It should have no effect, as they are only generated when they are accessed,
and as previously return a list then returning a subclass of list with two
overridden methods should not cause any large increase in memory usage.
 
> What is wrong with has_version()? You moved a lot of properties to the
> Version class, so I don't see any danger of cluttering the Package class
> with additional methods. Especially since it would make accessing the
> versions easier.
> 
> What should happen in the case of requesting a not provided version?
> You would have to raise an KeyError.
> 
> try:
>    pkg.versions["242-1"]
> except:
>    blaaa
> 
> I would prefer a simple "if pkg.has_version("242-1"): blabla".
Try/except is faster and the preferred form in the Python community, as
far as I know. EAFP - it's "Easier to Ask for Forgiveness than Permission".

When we use the get_version() way we should introduce an exception based
on LookupError, eg class NoVersionError(LookupError).

> 
> Finally I don't like mixing types. Too much confusion. The API should be
> easy to understand without having to look at the underlying code.
We could also introduce a version_map which would generate the requested
versions on access.

I should have used a dictionary for versions, but I used a list instead (not
even a generator, but a real list, which is not a very good idea for some
use cases).



-- 
Julian Andres Klode  - Free Software Developer
   Debian Developer  - Contributing Member of SPI
   Ubuntu Member     - Fellow of FSFE

Website: http://jak-linux.org/   XMPP: juliank@jabber.org
Debian:  http://www.debian.org/  SPI:  http://www.spi-inc.org/
Ubuntu:  http://www.ubuntu.com/  FSFE: http://www.fsfe.org/

Attachment: signature.asc
Description: Digital signature


Reply to: