Opened 5 years ago
Closed 5 years ago
#23419 closed enhancement (fixed)
New function can_assign_class() to partially replace is_extension_type()
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | cython | Keywords: | |
Cc: | nthiery, dkrenn | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | d6ab4da (Commits, GitHub, GitLab) | Commit: | d6ab4daa85d73700d7a6d0f5eb23d302c671675d |
Dependencies: | Stopgaps: |
Description (last modified by )
The semantics of both Python types and Cython types have changed a lot since is_extension_type()
was first written. In order to make it more clear why is_extension_type()
is used, I suggest to replace it by more specific functions.
In this ticket, I implement a new function can_assign_class(obj)
to determine whether obj.__class__
can be assigned to.
This also changes underlying_class
from a function to a method of CategoryObject
.
Change History (10)
comment:1 Changed 5 years ago by
- Cc dkrenn added
- Description modified (diff)
comment:2 Changed 5 years ago by
- Branch set to u/jdemeyer/new_function_can_assign_class___to_partially_replace_is_extension_type__
comment:3 Changed 5 years ago by
- Commit set to 6cb44f89a4478d711ba829d2cea0f8548623f989
- Status changed from new to needs_review
comment:4 Changed 5 years ago by
- Description modified (diff)
comment:5 Changed 5 years ago by
- Dependencies #23413 deleted
comment:6 follow-up: ↓ 7 Changed 5 years ago by
I think we should hide _underlying_class
from the public interface (well, specifically tab-completion).
Also, why the change in the test order?
- if not issubclass(self.__class__, Sets_parent_class) and not is_extension_type(self.__class__): + if can_assign_class(self) and not issubclass(self.__class__, Sets_parent_class):
(I don't object, but I am curious if there is a reason.)
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Replying to tscrim:
Also, why the change in the test order?
Efficiency. Checking can_assign_class
is very fast, so it's good to do that first.
comment:8 Changed 5 years ago by
- Commit changed from 6cb44f89a4478d711ba829d2cea0f8548623f989 to d6ab4daa85d73700d7a6d0f5eb23d302c671675d
Branch pushed to git repo; I updated commit sha1. New commits:
d6ab4da | Make _underlying_class private
|
comment:9 Changed 5 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Thanks.
comment:10 Changed 5 years ago by
- Branch changed from u/jdemeyer/new_function_can_assign_class___to_partially_replace_is_extension_type__ to d6ab4daa85d73700d7a6d0f5eb23d302c671675d
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Implement wrapperdescr_call without checking
Wording
Move various things to src/sage/cpython
New function can_assign_class()