Mailing List Archive: Zope: Dev

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained``

Index | Next | Previous | View Threaded

mh at gocept

May 14, 2009, 11:46 PM

Post #1 of 9 (1003 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained``

Am 15.05.2009 um 05:32 schrieb Chris McDonough:

> Log message for revision 99961:
> - Remove a dependency on ``zope.container.contained.Contained``
> (this is a dumb base class that defines __parent__ and __name__
> as None and declares that the class implements IContained).

What's the reason behind this refactoring?
Instead of zope.container.contained.Contained the IntIds class now
depends on zope.container.interface.IContained plus it redefines the
things which are already defined in
zope.container.contained.Contained. I do not see why this is better
than using the base class.

> Changed:
> U zope.intid/trunk/CHANGES.txt
> U zope.intid/trunk/src/zope/intid/__init__.py
[...]

> Modified: zope.intid/trunk/src/zope/intid/__init__.py
> =====================================================
> --- zope.intid/trunk/src/zope/intid/__init__.py 2009-05-15 02:41:49
> UTC (rev 99960)
> +++ zope.intid/trunk/src/zope/intid/__init__.py 2009-05-15 03:31:59
> UTC (rev 99961)
> @@ -24,10 +24,9 @@
>
> import BTrees
> from persistent import Persistent
> -from ZODB.interfaces import IConnection
> from zope.component import adapter, getAllUtilitiesRegisteredFor,
> subscribers
> from zope.container.interfaces import IObjectAddedEvent,
> IObjectRemovedEvent
> -from zope.container.contained import Contained
> +from zope.container.interfaces import IContained
> from zope.event import notify
> from zope.interface import implements
> from zope.keyreference.interfaces import IKeyReference, NotYet
> @@ -37,14 +36,16 @@
> from zope.intid.interfaces import IIntIds, IIntIdEvent
> from zope.intid.interfaces import IntIdAddedEvent, IntIdRemovedEvent
>
> -class IntIds(Persistent, Contained):
> +class IntIds(Persistent):
> """This utility provides a two way mapping between objects and
> integer ids.
>
> IKeyReferences to objects are stored in the indexes.
> """
> - implements(IIntIds)
> + implements(IIntIds, IContained)
>
> + __parent__ = __name__ = None
> +
> _v_nextid = None

Yours sincerely,
--
Michael Howitz · mh [at] gocept · software developer
gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany
http://gocept.com · tel +49 345 1229889 8 · fax +49 345 1229889 1
Zope and Plone consulting and development

_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

chrism at plope

May 14, 2009, 11:57 PM

Post #2 of 9 (958 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

On 5/15/09 2:46 AM, Michael Howitz wrote:
> Am 15.05.2009 um 05:32 schrieb Chris McDonough:
>
>> Log message for revision 99961:
>> - Remove a dependency on ``zope.container.contained.Contained``
>> (this is a dumb base class that defines __parent__ and __name__
>> as None and declares that the class implements IContained).
>
> What's the reason behind this refactoring?
> Instead of zope.container.contained.Contained the IntIds class now
> depends on zope.container.interface.IContained plus it redefines the
> things which are already defined in zope.container.contained.Contained.
> I do not see why this is better than using the base class.

It's a partial step towards getting rid of a dependency that zope.intid has on
zope.container. I'm thinking that maybe that IContained interface belongs in
some other package (e.g. maybe zope.contained). That Container base class is..
uh.. not complicated, so even if we never do get rid of the zope.container
dependency completely, it really doesn't harm anything to not use Contained.
Unless you have some nostalgia for it. ;-)

- C
_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

fdrake at gmail

May 15, 2009, 3:27 AM

Post #3 of 9 (945 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

On Fri, May 15, 2009 at 2:57 AM, Chris McDonough <chrism [at] plope> wrote:
> It's a partial step towards getting rid of a dependency that zope.intid has on
> zope.container. I'm thinking that maybe that IContained interface belongs in
> some other package (e.g. maybe zope.contained). That Container base class is..
> uh.. not complicated, so even if we never do get rid of the zope.container
> dependency completely, it really doesn't harm anything to not use Contained.
> Unless you have some nostalgia for it. ;-)

At the rate we're going, every class and every interface is going to
be in a separate package.

Keeping the dependency graph clean is great, and there's plenty to do
there. But there's also something to be said about being able to keep
a substantial portion of it in your head. The cleanliness of the
graph isn't so important if most users still can't understand just
because there are so many pieces that they wouldn't normally use
directly.

-Fred

--
Fred L. Drake, Jr. <fdrake at gmail.com>
"Chaos is the score upon which reality is written." --Henry Miller
_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

faassen at startifact

May 15, 2009, 4:48 AM

Post #4 of 9 (952 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

Chris McDonough wrote:
> On 5/15/09 2:46 AM, Michael Howitz wrote:
>> Am 15.05.2009 um 05:32 schrieb Chris McDonough:
>>
>>> Log message for revision 99961:
>>> - Remove a dependency on ``zope.container.contained.Contained``
>>> (this is a dumb base class that defines __parent__ and __name__
>>> as None and declares that the class implements IContained).
>> What's the reason behind this refactoring?
>> Instead of zope.container.contained.Contained the IntIds class now
>> depends on zope.container.interface.IContained plus it redefines the
>> things which are already defined in zope.container.contained.Contained.
>> I do not see why this is better than using the base class.
>
> It's a partial step towards getting rid of a dependency that zope.intid has on
> zope.container. I'm thinking that maybe that IContained interface belongs in
> some other package (e.g. maybe zope.contained). That Container base class is..
> uh.. not complicated, so even if we never do get rid of the zope.container
> dependency completely, it really doesn't harm anything to not use Contained.
> Unless you have some nostalgia for it. ;-)

Agreed with Chris; IContained interface is very minor and it's easy
enough to reimplement. If that helps us reduce dependencies I say let's
not worry about this step.

We might consider moving IContained to zope.location - it just
subclasses from ILocation after all and doesn't add anything besides
being a marker interface. zope.intid already depends on zope.location.
The Contained implementation could even move to zope.location, actually.

Hm, I wonder what code actually *depends* on IContained (instead
implements it).

Thanks Michael for watching the checkins carefully. Do keep bringing up
whatever issue you see here and we'll discuss it. Even if the checkin
stands, it can lead to useful discussions nonetheless.

Regards,

Martijn

_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

faassen at startifact

May 15, 2009, 5:15 AM

Post #5 of 9 (954 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

Fred Drake wrote:
> On Fri, May 15, 2009 at 2:57 AM, Chris McDonough <chrism [at] plope> wrote:
>> It's a partial step towards getting rid of a dependency that zope.intid has on
>> zope.container. I'm thinking that maybe that IContained interface belongs in
>> some other package (e.g. maybe zope.contained). That Container base class is..
>> uh.. not complicated, so even if we never do get rid of the zope.container
>> dependency completely, it really doesn't harm anything to not use Contained.
>> Unless you have some nostalgia for it. ;-)
>
> At the rate we're going, every class and every interface is going to
> be in a separate package.

I don't think we have many examples of this. We have zope.browser as a
repository of some interfaces. The other such package off the top of my
head is zope.broken, and that one really isn't pulling its weight so I
hope the IBroken interface can eventually move elsewhere. (someone,
analysis please? perhaps we need a package to hold content-specific
interfaces, equivalent to zope.browser. IBroken and IContained might
move there.. but see below)

There hasn't been a lot of splitting off of classes into new packages as
far as I know. We moved a lot from zope.app into zope. to leave the ZMI
behind, but I don't think that's been a bad exercise at all.

> Keeping the dependency graph clean is great, and there's plenty to do
> there. But there's also something to be said about being able to keep
> a substantial portion of it in your head.

Those two goals are not mutually exclusive and in fact mutually
supportive. It's not like it was easy to keep a portion in your head
before this work started anyway - it was a horrible, horrible, horrible
mess.

http://faassen.n--tree.net/blog/view/weblog/2009/01/29/0

I'd argue it's easier now that we can actually *read* the graphs. :)

> The cleanliness of the
> graph isn't so important if most users still can't understand just
> because there are so many pieces that they wouldn't normally use
> directly.

I appreciate that point. We shouldn't be generating a lot of small
pieces if we can help it. That's why it is important as a first impulse
to look at existing packages to move an interface into. Sometimes a
dependency inversion is the right way forward.

I think in the case of IContained it wouldn't hurt us much moving this
interface to zope.location, as IContained is just a marker. It's not as
clean as we'd like, but the dependency graph will be much cleaner if we
do and it's not horrible either.

We want both less packages and cleaner dependencies. I don't think we're
moving in the wrong direction at present though, so I don't think it's
the time to pull on the package-generation breaks just yet.

Regards,

Martijn

_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

srichter at cosmos

May 15, 2009, 6:06 AM

Post #6 of 9 (950 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

On Friday 15 May 2009, Fred Drake wrote:
> Keeping the dependency graph clean is great, and there's plenty to do
> there. But there's also something to be said about being able to keep
> a substantial portion of it in your head. The cleanliness of the
> graph isn't so important if most users still can't understand just
> because there are so many pieces that they wouldn't normally use
> directly.

Yes, I have voiced that concern several times myself. I personally do not even
understand where all this fear of installing many packages comes from. I
think it is because the ZCML is pulling in too many default registrations and
people are afraid of those, which I understand. But to create new packages
just because you do not want to use other parts of it is silly.

Regards,
Stephan
--
Entrepreneur and Software Geek
Google me. "Zope Stephan Richter"
_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

faassen at startifact

May 15, 2009, 6:55 AM

Post #7 of 9 (947 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

Stephan Richter wrote:
> On Friday 15 May 2009, Fred Drake wrote:
>> Keeping the dependency graph clean is great, and there's plenty to do
>> there. But there's also something to be said about being able to keep
>> a substantial portion of it in your head. The cleanliness of the
>> graph isn't so important if most users still can't understand just
>> because there are so many pieces that they wouldn't normally use
>> directly.
>
> Yes, I have voiced that concern several times myself. I personally do not even
> understand where all this fear of installing many packages comes from. I
> think it is because the ZCML is pulling in too many default registrations and
> people are afraid of those, which I understand. But to create new packages
> just because you do not want to use other parts of it is silly.

That's not silly at all.

I'm not afraid of installing many packages for my applications. But for
libraries and little frameworks, I don't like having to depend on 70
other packages even though my library isn't using 99.9% of the code in
there in any way. Is that silly?

We created zope.container because we don't want to use all the code in
zope.app.container. zope.app.container contains the ZMI a lot of code we
don't want to be there in our apps as we're not intending to use it. Is
that silly?

The zope.browser package was created to prevent, among other things,
z3c.form from pulling in code from zope.formlib, a completely separate
form library. But it wasn't a good situation. People actually got
confused and thought they had something to do with each other, in that
z3c.form uses zope.formlib, which it doesn't. Is preventing that
situation silly?

The principle has to do more with *less code* than *less packages*.
We're trying to make it possible to use and be aware of less code when
considering individual chunks of the code base. To create loose coupling
so that you don't have to worry about all chunks of the codebase even
though you're only concerned with a little bit of it. To get rid of the
code that isn't used a lot (such as the ZMI).

If we can make our zope.* packages not rely on a huge amount of other
packages that they aren't really using anyway, we stand a larger change
understanding them ourselves, and we stand a larger change others might
understand them too and adopt them.

I could understand these concerns with package creation better if people
would point out how the total amount of packages installed for an
application or library is increasing (once the applications and
libraries have been adjusted to import from the new locations). But I
think the amount of packages is decreasing...

Regards,

Martijn

_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

chrism at plope

May 15, 2009, 9:20 AM

Post #8 of 9 (949 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

On 5/15/09 6:27 AM, Fred Drake wrote:
> On Fri, May 15, 2009 at 2:57 AM, Chris McDonough<chrism [at] plope> wrote:
>> It's a partial step towards getting rid of a dependency that zope.intid has on
>> zope.container. I'm thinking that maybe that IContained interface belongs in
>> some other package (e.g. maybe zope.contained). That Container base class is..
>> uh.. not complicated, so even if we never do get rid of the zope.container
>> dependency completely, it really doesn't harm anything to not use Contained.
>> Unless you have some nostalgia for it. ;-)
>
> At the rate we're going, every class and every interface is going to
> be in a separate package.
>
> Keeping the dependency graph clean is great, and there's plenty to do
> there. But there's also something to be said about being able to keep
> a substantial portion of it in your head. The cleanliness of the
> graph isn't so important if most users still can't understand just
> because there are so many pieces that they wouldn't normally use
> directly.

This particular changes is a nobrainer. It replaces a base class that defines
__parent__ and __name__ as None. It just can't be more understandable to have
IntIds subclass from Contained for a new developer. When it subclasses from
Contained, they have go look over there to see if it does anything interesting
and it just doesn't.

Breaking small import dependencies like this one is useful even if you don't
manage to break any full package dependencies, if only to get the "little stuff"
out of the way to start thinking about whether it makes sense to do anything
larger. I *didn't* make any other changes because I don't know what the right
thing is wrt interfaces and such, and I don't want to make things even worse.

- C
_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

tseaver at palladion

May 15, 2009, 10:01 AM

Post #9 of 9 (953 views)
Permalink

Re: [Checkins] SVN: zope.intid/trunk/ - Remove a dependency on ``zope.container.contained.Contained`` [In reply to]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martijn Faassen wrote:

> We might consider moving IContained to zope.location - it just
> subclasses from ILocation after all and doesn't add anything besides
> being a marker interface. zope.intid already depends on zope.location.
> The Contained implementation could even move to zope.location, actually.
>
> Hm, I wonder what code actually *depends* on IContained (instead
> implements it).

+1 to moving IContained into zope.location.

+lots to removing it altogether, if possible: I'm afraid that there are
going to be a few intractable component registrations against it "in the
wild", however.

Tres.
- --
===================================================================
Tres Seaver +1 540-429-0999 tseaver [at] palladion
Palladion Software "Excellence by Design" http://palladion.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFKDZ/Y+gerLs4ltQ4RAnnEAKCaXlLL1wsA1RUJeUTWwj98DNzeqQCdH2Av
uSwUKR0xsNw7H411LagHneY=
=WbeS
-----END PGP SIGNATURE-----

_______________________________________________
Zope-Dev maillist - Zope-Dev [at] zope
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads