Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix .abstract for classes named with leading underscore(s) #439

Merged
merged 2 commits into from Oct 22, 2020

Conversation

ceball
Copy link
Member

@ceball ceball commented Sep 18, 2020

Fix the (longstanding) assumption in param's abstract class mechanism that abstract classes don't start with a leading underscore.

Any identifier of the form __spam (at least two leading underscores, at most one trailing underscore) is textually replaced with _classname__spam, where classname is the current class name with leading underscore(s) stripped. This mangling is done without regard to the syntactic position of the identifier, as long as it occurs within the definition of a class.

(https://docs.python.org/2/tutorial/classes.html)

param/param/parameterized.py

Lines 2074 to 2094 in 301b961

# Python 2.6 added abstract base classes; see
# https://github.com/ioam/param/issues/84
def __is_abstract(mcs):
"""
Return True if the class has an attribute __abstract set to True.
Subclasses will return False unless they themselves have
__abstract set to true. This mechanism allows a class to
declare itself to be abstract (e.g. to avoid it being offered
as an option in a GUI), without the "abstract" property being
inherited by its subclasses (at least one of which is
presumably not abstract).
"""
# Can't just do ".__abstract", because that is mangled to
# _ParameterizedMetaclass__abstract before running, but
# the actual class object will have an attribute
# _ClassName__abstract. So, we have to mangle it ourselves at
# runtime.
try:
return getattr(mcs,'_%s__abstract'%mcs.__name__)
except AttributeError:
return False

(i.e. param does not do the same thing as python)

(originally from #423 (comment))

Also added note about API0 and API1 tests (although not going so far as to say when you should be adding to one or the other or both, just noting the current duplication...)

@ceball ceball requested a review from jbednar September 18, 2020 08:19
@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.447% when pulling 8c1b811 on abstract_fix into 301b961 on master.

@philippjfr
Copy link
Member

Looks good to me!

@philippjfr philippjfr merged commit adb0641 into master Oct 22, 2020
Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@jbednar jbednar deleted the abstract_fix branch October 23, 2020 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants