Opened 9 years ago
Closed 9 years ago
#25069 closed New feature (wontfix)
Make _unregister_lookup be public API
Reported by: | Andriy Sokolovskiy | Owned by: | Andriy Sokolovskiy |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Anssi Kääriäinen | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
(as a follow-up for discussion from https://github.com/django/django/pull/4948)
Django allows users to register a custom lookups, but there are no public way to unregister it.
I don't see a reason to keep _unregister_lookup
as a private API, so I'm proposing to make it be public and add documentation for it.
Change History (7)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Well, my vision (in abstract) is if there is a public API to produce some changes (for this case to add new entity to existing collection) there should be a way to revert this change (for this case to remove entity from collection).
comment:3 by , 9 years ago
We can wait for Anssi to say if there's any more explanation for making it private other than the existing comment, "Meant to be used in tests only," but I don't see much advantage to making a public method that doesn't have a concrete use case.
comment:4 by , 9 years ago
Well, I prefer to remove it at all :) I don't see a reason to have method in codebase, which is used only in tests. We can avoid it as I described it in comment of my PR.
Simon's opinion is that this method is needed.
So, I think if we will leave it, it should be made public.
comment:5 by , 9 years ago
My point is that I'd rather have a method (private or public) that deals with the implementation details of lookup unregistration than moving this logic in the tests by directly assigning to class_looukps
.
comment:6 by , 9 years ago
Cc: | added |
---|
comment:7 by , 9 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Do you have a use case in mind?