Opened 7 years ago

Closed 7 years ago

#6691 closed defect (fixed)

BaseFilter isn't really an abstract class... yet

Reported by: neena Owned by: gsathya
Priority: Very Low Milestone:
Component: Metrics/Compass Version:
Severity: Keywords:
Cc: karsten Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

import compass
class Foo(compass.BaseFilter): pass

f=Foo()
f.accept(None)

Works fine. It shouldn't.

Attaching a patch which fixes this.

Child Tickets

Attachments (1)

0001-Make-BaseFilter-an-abstract-class.patch (795 bytes) - added by neena 7 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 in reply to:  description ; Changed 7 years ago by gsathya

Owner: changed from neena to gsathya
Status: newassigned

I wonder if abstract classes even make sense in a dynamically typed language. I'd be all for removing @abstractmethod instead of adding more abstract stuff. What do you think?

comment:2 in reply to:  1 ; Changed 7 years ago by neena

Replying to gsathya:

I wonder if abstract classes even make sense in a dynamically typed language. I'd be all for removing @abstractmethod instead of adding more abstract stuff. What do you think?

I would add the metaclass. Though, I don't feel too strongly about it.

comment:3 Changed 7 years ago by gsathya

Cc: karsten added; gsathya removed

comment:4 in reply to:  2 Changed 7 years ago by karsten

Replying to neena:

Replying to gsathya:

I wonder if abstract classes even make sense in a dynamically typed language. I'd be all for removing @abstractmethod instead of adding more abstract stuff. What do you think?

I would add the metaclass. Though, I don't feel too strongly about it.

Hmm, why am I cc'ed? To me as a non-Pythonian, Sathya's statement makes sense. But maybe that's because I don't know what __metaclass__ does. Anyway, I think I'll leave this ticket to the Python developers and will review/merge whatever Sathya comes up with. :)

comment:5 Changed 7 years ago by gsathya

Status: assignedneeds_review

Please review my bug_6691 branch. Thanks!

comment:6 in reply to:  5 Changed 7 years ago by neena

Replying to gsathya:

Please review my bug_6691 branch. Thanks!

Looks good.

comment:7 Changed 7 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Cool, merged! Closing.

Note: See TracTickets for help on using tickets.