python / cpython

The Python programming language
https://www.python.org/
Other
61.25k stars 29.55k forks source link

@public - an __all__ decorator #70819

Closed warsaw closed 8 years ago

warsaw commented 8 years ago
BPO 26632
Nosy @warsaw, @rhettinger, @ncoghlan, @bitdancer, @ethanfurman, @berkerpeksag, @vadmium, @zware, @eryksun, @leewz, @jayvdb
Files
  • 26632-in-c.diff
  • 26632-in-c-2.diff
  • 26632-in-c-3.diff
  • 26632-in-c-4.diff
  • 26632-in-c-5.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = created_at = labels = ['interpreter-core', 'type-feature'] title = '@public - an __all__ decorator' updated_at = user = 'https://github.com/warsaw' ``` bugs.python.org fields: ```python activity = actor = 'ncoghlan' assignee = 'none' closed = True closed_date = closer = 'barry' components = ['Interpreter Core'] creation = creator = 'barry' dependencies = [] files = ['42757', '42758', '42760', '42784', '42833'] hgrepos = [] issue_num = 26632 keywords = ['patch'] message_count = 42.0 messages = ['262319', '262320', '262321', '262346', '262380', '262383', '262389', '262390', '262396', '262562', '262564', '262568', '262575', '262617', '262697', '262878', '264977', '264980', '264987', '264997', '265001', '265165', '265198', '265207', '265211', '265428', '265910', '265911', '266107', '266108', '266117', '266120', '266152', '266159', '266319', '266403', '266411', '266746', '266756', '267305', '267734', '285092'] nosy_count = 11.0 nosy_names = ['barry', 'rhettinger', 'ncoghlan', 'r.david.murray', 'ethan.furman', 'berker.peksag', 'martin.panter', 'zach.ware', 'eryksun', 'leewz', 'jayvdb'] pr_nums = [] priority = 'normal' resolution = 'wont fix' stage = 'patch review' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue26632' versions = ['Python 3.6'] ```

    warsaw commented 8 years ago

    This is probably terrible, but given how difficult it is to keep __all__'s up to date, maybe something like this can be useful. I literally whipped this up in about 5 minutes, so sit back and watch the bikeshedding!

    import sys
    
    def public(thing):
        if isinstance(thing, str):
            mdict = sys._getframe(1).f_globals
            name = thing
        else:
            mdict = sys.modules[thing.__module__].__dict__
            name = thing.__name__
        dunder_all = mdict.setdefault('__all__', [])
        dunder_all.append(name)
        return thing

    Then:

    @public
    def baz(a, b):
        return a + b
    
    @public
    def buz(c, d):
        return c / d
    
    def qux(e, f):
        return e * f
    
    class zup:
        pass
    
    @public
    class zap:
        pass
    
    public('CONST1')
    CONST1 = 3
    
    CONST2 = 4
    
    public('CONST3')
    CONST3 = 5

    Normally for any callable with an __name, you can just decorate it with @public to add it to __all. Of course that doesn't worth with things like constants, thus the str argument. But of course that also requires sys._getframe() so blech.

    warsaw commented 8 years ago

    Oh, and it should be a built-in \<wink>

    ethanfurman commented 8 years ago
    def public(thing, value=None):
        if isinstance(thing, str):
            mdict = sys._getframe(1).f_globals
            name = thing
            mdict[name] = thing  # no need for retyping! ;)
        else:
            mdict = sys.modules[thing.__module__].__dict__
            name = thing.__name__
        dunder_all = mdict.setdefault('__all__', [])
        dunder_all.append(name)
        return thing
    
    @public
    def baz(a, b):
        return a+ b
    
    public('CONST1', 3)
    
    CONST2 = 4

    On the down side, you know somebody is going to @public a class' method -- how do we check for that?

    warsaw commented 8 years ago

    On Mar 24, 2016, at 02:48 AM, Ethan Furman wrote:

    On the down side, you know somebody is going to @public a class' method -- how do we check for that?

    Do we need to? Consenting adults and __all__.

    OT1H, you do get an AttributeError if you from-import-* and there are things in __all__ that aren't in the module. OTOH, it's a pretty obvious error.

    vadmium commented 8 years ago

    FWIW I already invented this :) as written in bpo-22247. Although I think I only used it once or twice in my own personal librarie(s). So it’s a nice sign that we came up with the same @public name and usage.

    I’m not a fan of hacks depending on the calling frame, and I prefer APIs that “only do one thing”. So I am okay with accepting an object and work off its __name and __module, but not okay with also accepting a string and guessing what module it was defined in.

    warsaw commented 8 years ago

    On Mar 24, 2016, at 10:31 PM, Martin Panter wrote:

    FWIW I already invented this :) as written in bpo-22247. Although I think I only used it once or twice in my own personal librarie(s). So it’s a nice sign that we came up with the same @public name and usage.

    Cool! Consider that bikeshed painted then. :)

    I’m not a fan of hacks depending on the calling frame, and I prefer APIs that “only do one thing”. So I am okay with accepting an object and work off its name and __module__, but not okay with also accepting a string and guessing what module it was defined in.

    Yes, but it makes it less convenient to add non-"APIs" to __all__, although I guess you can just append it at the point of use:

    __all__.append('CONST1')
    CONST1 = 3

    Not as pretty, and now you have two ways of doing it.

    Here's another thought:

    What if we gave all modules an __all__ automatically, and that was an object that acted like a list but also had an @public decorator.

    import sys
    
    class All(list):
        def public(self, api):
            sys.modules[api.__module__].__all__.append(api.__name__)
    
    __all__ = All()
    
    @__all__.public
    def foo():
        pass
    
    @__all__.public
    class Bar:
        pass
    
    __all__.append('CONST')
    CONST = 1
    
    print(__all__)
    ethanfurman commented 8 years ago

    Not a fan. :/

    How about getting your own copy of the public decorator initialized with the globals you pass in?

    class Public:
        def __init__(self, module):
            """
            module should be the globals() dict from the calling module
            """
            self.module = module
            self.module.setdefault('__all__', [])
        def __call__(self, thing, value=None):
            if isinstance(thing, str):
                self.module[thing] = value
            else:
                self.module[thing.__name__] = thing

    and in use:

    public = Public(globals())
    
    @public
    def baz(a, b):
        #blah blah
    
    public('CONST1', 2)
    warsaw commented 8 years ago

    On Mar 25, 2016, at 12:14 AM, Ethan Furman wrote:

    public = Public(globals())

    @public def baz(a, b):

    blah blah

    public('CONST1', 2)

    I'm not crazy about that, plus I rather don't like the implicit binding of the name. I suppose we should just drop the idea of convenience for non-"API". Just use the defined @public for classes and functions, and an explicit __all__.append('CONST') for other names.

    eryksun commented 8 years ago

    work off its __name and __module

    Why is __module__ required? It seems to me this should only operate on the current module.

    I added a prototype to Python/bltinmodule.c that gets or creates the __all list from the current globals (i.e. PyEvalGetGlobals). It accepts at most one positional argument and any number of keyword arguments. It adds the positional argument's \_name to __all, sets it in globals, and returns a reference for use as a decorator. The keyword argument dict is used to update globals and extend __all.

        Python 3.6.0a0 (default:3eec7bcc14a4+, Mar 24 2016, 20:40:52) 
        [GCC 4.8.4] on linux
        Type "help", "copyright", "credits" or "license" for more
        information.
        >>> @public
        ... def foo():
        ...     pass
        ... 
        >>> def bar(): pass
        ... 
        >>> public(bar, spam=1, eggs=2)
        <function bar at 0x7efe96ca1048>
        >>> __all__
        ['foo', 'spam', 'eggs', 'bar']
        >>> foo, bar
        (<function foo at 0x7efe96c8af28>, <function bar at 0x7efe96ca1048>)
        >>> spam, eggs
        (1, 2)

    Maybe it should be generalized to handle multiple positional arguments. Currently it's an error:

        >>> public(foo, bar)
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        TypeError: public expected at most 1 arguments, got 2

    The positional argument must have a __name__ that's a string:

        >>> public('CONST')
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        AttributeError: 'str' object has no attribute '__name__'
        >>> class C:
        ...     __name__ = 1
        ... 
        >>> public(C())
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
        TypeError: __name__ must be a string

    If it's used to decorate a method definition, it stores a reference to the function in the module's globals. That's not very useful, but at least it won't lead to an error with a star import.

    rhettinger commented 8 years ago

    [Barry]

    This is probably terrible ...

    I have to agree with that part ;-) Sorry, but this feels "yucky" and is likely to cause more problems than it solves.

    serhiy-storchaka commented 8 years ago

    Agree with Raymond.

    warsaw commented 8 years ago

    On Mar 28, 2016, at 06:34 AM, Raymond Hettinger wrote:

    I have to agree with that part ;-) Sorry, but this feels "yucky" and is likely to cause more problems than it solves.

    I've been experimenting with something like this in a Mailman branch and I've come to like it much more than I did originally. I'm using the "simple" implementation, so that means that I have a very few explicit appends to __all__.

    But the use of @public is actually pretty great. I don't have to worry about __all__ getting out of sync (and there turned out to be lots of places where that was happening), it's quite visually appealing (easy to pick out in a crowded file), and it avoids nasty PEP-8 conflicts.

    The major downside is actually having to import it from a module very early in the startup sequence. I stick it in mailman/init.py but that kind of sucks because I want to move to making that a namespace package so I want to remove mailman/init.py but there's no other obvious place to put an early public definition.

    Thus after experimenting with it, I'm much more in favor of it. Could you please describe what you don't like about it and what problems you think it will cause?

    (Plus, I encourage you to try it on a medium to large size project!)

    serhiy-storchaka commented 8 years ago

    There is a helper in test.support that helps to add a test for __all__, and we slowly add these test for all modules. So this is not large issue for the stdlib.

    New module level names are added not very often. Keeping __all__ in sync is not the largest problem in the maintaining.

    warsaw commented 8 years ago

    On Mar 28, 2016, at 04:26 PM, Serhiy Storchaka wrote:

    There is a helper in test.support that helps to add a test for __all__, and we slowly add these test for all modules. So this is not large issue for the stdlib.

    New module level names are added not very often. Keeping __all__ in sync is not the largest problem in the maintaining.

    stdlib use is not really the point of this proposal. It's for all those 3rd party projects that use __all__.

    warsaw commented 8 years ago

    On Mar 25, 2016, at 02:12 AM, Eryk Sun wrote:

    I added a prototype to Python/bltinmodule.c that gets or creates the __all__ list from the current globals (i.e. PyEval_GetGlobals).

    Hi Eryk. Can you post your diff to bltinmodule.c? I'd like to see your code.

    It accepts at most one positional argument and any number of keyword arguments. It adds the positional argument's __name to __all, sets it in globals, and returns a reference for use as a decorator. The keyword argument dict is used to update globals and extend __all__.

    I like this. The dual functionality of public looks like it will handle almost all use cases. I think we're in widespread agreement about the decorator, and the keyword arguments are a nice approach to public constants.

    I guess I'm a little less sure about the positional argument API. In adding @public to Mailman, I noticed there are a few public names which are instances. These could be "publicized" with the keyword argument approach, but I don't think they can work as positional arguments, because the instances themselves don't have __name__s. For example, currently:

    factory = DateFactory()
    factory.reset()
    today = factory.today
    now = factory.now
    layers.MockAndMonkeyLayer.register_reset(factory.reset)
    
    __all__.extend([
        'factory',
        'now',
        'today',
        ])

    With only keyword arguments, which isn't bad:

    public(factory=DateFactory())
    factory.reset()
    public(today=factory.today, now=factor.now)

    What's the use case for positionals?

    The positional argument must have a __name__ that's a string:

    Right. But what class of objects does that cover that isn't already covered (or that explicitly appending to __all__ is good enough)?

    warsaw commented 8 years ago

    Here's my implementation based on eryksun's idea:

    def public(thing=None, **kws):
        mdict = (sys._getframe(1).f_globals
                 if thing is None
                 else sys.modules[thing.__module__].__dict__)
        dunder_all = mdict.setdefault('__all__', [])
        if thing is not None:
            dunder_all.append(thing.__name__)
        for key, value in kws.items():
            dunder_all.append(key)
            mdict[key] = value
        return thing
    warsaw commented 8 years ago

    Here's a C implementation. I'm a bit under the weather so please do double check my refcounting logic. ;)

    No tests or docs yet, but those would be easy to add. Here's an example:

    @public
    class Foo:
        pass
    
    public(qux=3)
    
    print(qux)
    
    @public
    def zzz():
        pass
    
    public(jix=1, jox=2, jrx=3)
    
    print(__all__)
    print(jix, jox, jrx)

    You could also try to add an explicit __all__ in the module and those names will get appended to it.

    warsaw commented 8 years ago

    I think I missed a decref. New diff.

    serhiy-storchaka commented 8 years ago

    Added a couple of comments on Rietveld.

    But sorry, the overall idea looks unpythonic to me. I'm strong -1.

    warsaw commented 8 years ago

    Updated.

    bitdancer commented 8 years ago

    "This will cause more problems than it solves" and "this looks unpythonic" are, IMO, not strong arguments against it without butressing discussion. If we can have some examples of problems it will cause, or a concrete explanation of wy something that makes code easier to understand and update (by putting the __all__ declaration next to the object being made public) is unpythonic, that would also help.

    warsaw commented 8 years ago

    One more diff. As I was working on tests, I realized that the decorator version wasn't returning the thing it was decorating. Changing this also allowed me to simplify the exit path.

    I should be putting up a PyPI package soon which implements this for earlier Pythons (Python 3 only for now though).

    warsaw commented 8 years ago

    Here's a standalone version for older Python 3's:

    http://public.readthedocs.io/en/latest/ https://pypi.python.org/pypi/atpublic

    ethanfurman commented 8 years ago

    For the standalone version I suggest a disclaimer about the from ... import * ability. Something like:

    from ... import * should not be used with packages that do not have an all unless they support that usage (check their docs).

    warsaw commented 8 years ago

    On May 09, 2016, at 04:19 PM, Ethan Furman wrote:

    from ... import * should not be used with packages that do not have an all unless they support that usage (check their docs).

    Good idea; I added a warning-ish.

    warsaw commented 8 years ago

    Now with docs and tests.

    berkerpeksag commented 8 years ago

    +1 for the idea. I saw a lot of different 'all' or 'public' decorators in the wild. It would be nice to have a complete solution in Python. It would be good to add a note to Doc/whatsnew/3.6.rst.

    fa5ec958-6532-42d7-99e4-e0654bdcb9f7 commented 8 years ago

    I like how @public keeps the declaration close to the definition.

    I am iffy about using public to define other values. That part might be considered unpythonic.

    Implementation issues:

    > On the down side, you know somebody is going to @public a class' method -- > how do we check for that?

    Do we need to? Consenting adults and __all__.

    It's a silent error waiting to happen. If you never use import * on it (e.g. because it's your main file), you won't get the error message. Things will work "as expected" (your methods are class-public!) until you give a method the same name as a builtin or something you imported or defined earlier. When that happens, the error message will have nothing to do with the problem.

    It might be detectable using thing.__qualname__ != thing.__name__, but this disallows functions decorated without updating qualname, and static/class methods exposed in a module's interface.

    It might be detectable by checking, on the callstack, whether you're in a module load or a class definition.

    Bikeshed \========

    How many public module values aren't enum-type constants? It could be useful to be able to dump an enum into a module's space. I mean, a canonical way. With that, maybe maintaining module-level constants in __all__ isn't that big a deal.

    # Rather than:
    globals().update(MyEnum.__members__)
    __all__.extend(MyEnum.__members__)
    # Perhaps allow:
    enum.dump_namespace(MyEnum, globals())

    About the cost paid at every load:

    API:

    P.S. Typo in the ReadTheDocs. py_install should be a function call, right?

    >>> from public import py_install
    >>> py_install

    P.S.: Would OrderedSet (which doesn't exist) be the ideal type for __all? I mean, if you had to use someone else's __all, not if you had to maintain it.

    warsaw commented 8 years ago

    On May 20, 2016, at 07:21 AM, Franklin? Lee wrote:

    I am iffy about using public to define other values. That part might be considered unpythonic.

    It's a bit of a stretch. I like it for the convenience, and the implementation is simple, but if e.g. Guido disliked this part of it, I'd be okay dropping it. I think the use on non-name'd things is rare enough that a little inconvenience wouldn't be a fatal flaw.

    • __module__ is not reliable. functools.wraps changes it. (Why
    • does it do that, though?)

    I don't know, but what practical effect will this have? I.e. under what conditions would you @public wrap a @functools.wraps function and want it to show up in __all__? Do you have a specific use case?

    Also note that this is a subtle difference between the C and Python implementations. I actually expect that if this were adopted for Python 3.6, we'd pretty much only use the C version. In the standalone package, I'm including the Python versions mostly just for convenience in environments without a compiler (though maybe a built wheel for some platforms would be useful).

    • If __all__ isn't a list, you'd have to make it a list before you mess
    • with it. (Is this possible?)

    It would be possible. I'd just do the equivalent of::

        __all__ = list(__all__)

    But I actually think that best practice would be not to set __all__ explicitly if you're going to use @public. If you really want it to be immutable, you'd put the following at the bottom of your module:

        __all__ = tuple(__all__)

    For now I've added some usage caveats that describe these points.

    On the down side, you know somebody is going to @public a class' method -- how do we check for that?

    Do we need to? Consenting adults and all.

    It's a silent error waiting to happen. If you never use import * on it (e.g. because it's your main file), you won't get the error message. Things will work "as expected" (your methods are class-public!) until you give a method the same name as a builtin or something you imported or defined earlier. When that happens, the error message will have nothing to do with the problem.

    It might be detectable using thing.__qualname__ != thing.__name__, but this disallows functions decorated without updating qualname, and static/class methods exposed in a module's interface.

    It might be detectable by checking, on the callstack, whether you're in a module load or a class definition.

    Sure, we could probably add some heuristics, but I still don't see the need for the extra complexity. The error will be far from the declaration, but the exception should make it relatively obvious what's going on. I also really don't think folks would naturally be inclined to put @public on anything but a top-level definition. You wouldn't ever put such a thing in your __all__ so why would you decorate it with @public?

    In any case, I've added a usage caveat for this case too.

    How many public module values aren't enum-type constants?

    These days I bet they are quite a bit more common than enum-types, although I agree that enums are great and we should use more of them! Just historically speaking I don't know how many packages have converted all their constants over to enums.

    Also, I know that I have several cases where constants are actually instances. They could be marker objects like::

        MARKER = object()

    or system globals::

        configuration = Configuration()

    I'd want both of those in __all__.

    It could be useful to be able to dump an enum into a module's space. I mean, a canonical way. With that, maybe maintaining module-level constants in all isn't that big a deal.

    Rather than:

    globals().update(MyEnum.__members) __all.extend(MyEnum.__members__)

    Perhaps allow:

    enum.dump_namespace(MyEnum, globals())

    It's an interesting thought.

    About the cost paid at every load:

    • Should tools update all for you, and comment out the @publics?
      • If so, how would they deal with public non-callable values?
    • When compiling to .pyc, should the compiler remove @public calls and explicitly add the values to all?

    Why? Aren't those one-time costs only borne when the module is originally imported?

    API:

    • Alternative syntax for constants, requiring less frame hackery: public(globals(), x=1, y=2, z=3)

    Possibly. Since this is really only relevant for the pure-Python implementation, I'm not as keen on the extra cruft.

    • Naming: Is it really "public"? Some names might be public but not in
    • __all__.

    What does it mean for a name to be "public but not in __all__"?

    I'll also note that since the basic API has been independently invented at least three times, and all of them use @public, it seems like the obvious choice. ;)

    P.S. Typo in the ReadTheDocs. py_install should be a function call, right?

    from public import py_install py_install

    Fixed, thanks!

    P.S.: Would OrderedSet (which doesn't exist) be the ideal type for __all? I mean, if you had to use someone else's __all, not if you had to maintain it.

    It's an interesting thought, but I don't know if it's enough of a use case to add collections.OrderedSet. Traditionally __all__ has been a list, with some relatively recent moves to making it a tuple.

    Thanks for the interesting and useful feedback!

    warsaw commented 8 years ago

    On May 20, 2016, at 07:02 AM, Berker Peksag wrote:

    +1 for the idea. I saw a lot of different 'all' or 'public' decorators in the wild. It would be nice to have a complete solution in Python. It would be good to add a note to Doc/whatsnew/3.6.rst.

    Is that a pronouncement? :)

    vadmium commented 8 years ago

    Here are two examples where publicly-documented module attributes are intentionally omitted from __all__:

    Despite these, I think @public is a reasonable name. But I may be biased, because I personally think everything should be included in __all__. Otherwise pydoc does not pick it up.

    warsaw commented 8 years ago

    On May 22, 2016, at 11:30 PM, Martin Panter wrote:

    Here are two examples where publicly-documented module attributes are intentionally omitted from __all__:

    • bpo-26234: typing.re and typing.io
    • bpo-23439: HTTP status codes like http.client.NOT_FOUND

    Wild.

    Despite these, I think @public is a reasonable name. But I may be biased, because I personally think everything should be included in all. Otherwise pydoc does not pick it up.

    I think it's pretty reasonable, and pretty well-established despite some exceptions, that __all__ names a module's public symbols.

    fa5ec958-6532-42d7-99e4-e0654bdcb9f7 commented 8 years ago

    I don't know, but what practical effect will this have? I.e. under what conditions would you @public wrap a @functools.wraps function and want it to show up in __all__? Do you have a specific use case?

    I sometimes wrap functions that return iterators to make functions that return lists, because I work on the interpreter a lot. From there, it's not much of a stretch to imagine functions which are implemented as decorated versions of other functions.

    If @public were only to be used as a decorator, it would not be possible to have public called on a function outside of its definition. But someone might call public(some_decorator(some_function)).

    (@public is really a macro, if you think about it.)

    It would be possible.

    (I meant, is it possible for someone to have a non-list __all__?)

    If the .append fails, I think there should be a meaningful error. Perhaps "'all' is not a list."

    Sure, we could probably add some heuristics, but I still don't see the need for the extra complexity. The error will be far from the declaration, but the exception should make it relatively obvious what's going on. I also really don't think folks would naturally be inclined to put @public on anything but a top-level definition. You wouldn't ever put such a thing in your __all__ so why would you decorate it with @public?

    I'm thinking of the people who don't read docs and are coming from other languages. They'd put @public over their method, and one day they'd import * from that file (whereas they used to only import explicitly), getting an error about a name not being defined in their module. "But why would that name need to be defined? It's a method."

    Or worse, the name of the method just happens to be the same as something in some other file, so they'll focus on why that NAME is being expected in THIS file.

    Also, I know that I have several cases where constants are actually instances. They could be marker objects like::

    MARKER = object()

    (Here's food for thought: A MARKER could be a one-element enum, both conceptually and by implementation. Just like how the "bool enum" is {True,False} and the "None enum" is {None}.)

    warsaw commented 8 years ago

    On May 23, 2016, at 02:43 PM, Franklin? Lee wrote:

    I sometimes wrap functions that return iterators to make functions that return lists, because I work on the interpreter a lot. From there, it's not much of a stretch to imagine functions which are implemented as decorated versions of other functions.

    If @public were only to be used as a decorator, it would not be possible to have public called on a function outside of its definition. But someone might call public(some_decorator(some_function)).

    Do you mean, they'd call this is some module other than the one some_function was defined in? I don't know that this is a use case we even want to support.

    (@public is really a macro, if you think about it.)

    That's true in a sense. It doesn't change the decorated thing at all. I think it's important to keep in mind that @public isn't the only way to add to __all__.

    >It would be possible.

    (I meant, is it possible for someone to have a non-list __all__?)

    Yes. I've seen existing code where __all__ is assigned to a tuple.

    If the .append fails, I think there should be a meaningful error. Perhaps "'all' is not a list."

    You should get something like:

    AttributeError: 'tuple' object has no attribute 'append'

    which seems pretty obvious.

    I'm thinking of the people who don't read docs and are coming from other languages. They'd put @public over their method, and one day they'd import * from that file (whereas they used to only import explicitly), getting an error about a name not being defined in their module. "But why would that name need to be defined? It's a method."

    Or worse, the name of the method just happens to be the same as something in some other file, so they'll focus on why that NAME is being expected in THIS file.

    Well, consenting adults and all. I'm not sure we need to protect ourselves so strictly against people who don't read the docs and don't understand Python (i.e. random cargo-culters).

    >Also, I know that I have several cases where constants are actually >instances. They could be marker objects like:: > > MARKER = object()

    (Here's food for thought: A MARKER could be a one-element enum, both conceptually and by implementation. Just like how the "bool enum" is {True,False} and the "None enum" is {None}.)

    Sure. I don't think that changes anything here though. Down the line, it might be an interesting idiom to experiment with (you can probably start with the standalone enum34 module).

    fa5ec958-6532-42d7-99e4-e0654bdcb9f7 commented 8 years ago

    If @public were only to be used as a decorator, it would not be possible to have public called on a function outside of its definition. But someone might call public(some_decorator(some_function)).

    Do you mean, they'd call this is some module other than the one some_function was defined in? I don't know that this is a use case we even want to support.

    I mean they'd define their own function as a wrapped version of another function.

    That's true in a sense. It doesn't change the decorated thing at all. I think it's important to keep in mind that @public isn't the only way to add to all.

    I mean more in that it acts in the scope of its caller, rather than its definition.

    You should get something like:

    AttributeError: 'tuple' object has no attribute 'append'

    which seems pretty obvious.

    I don't think the C version shows a traceback, so it won't be clear that you're trying to assign to __all__.

    When I rewrote public from memory, I wrote it something like this: try: dunder_all.append(name) except TypeError: module.all = [*dunder_all, name]

    Well, consenting adults and all. I'm not sure we need to protect ourselves so strictly against people who don't read the docs and don't understand Python (i.e. random cargo-culters).

    Python is a popular learning language, and many will be students who haven't yet trained to reflexively look up docs. I saw the lack of such habits in Python's IRC channel.

    "Consenting adults", I feel, is a reason to grant power: don't stop people from doing something they might need to do. But @public on a class method is just an error.

    warsaw commented 8 years ago

    On May 25, 2016, at 07:56 AM, Franklin? Lee wrote:

    AttributeError: 'tuple' object has no attribute 'append' which seems pretty obvious.

    I don't think the C version shows a traceback, so it won't be clear that you're trying to assign to __all__.

    Thanks! I'll fix both the C and Python versions to raise a ValueError when __all__ isn't a list. I'll make the change in my atpublic PyPI project (version 0.3) and will update the CPython patch if and when it gets accepted.

    I think for now at least I won't autoconvert it to a list. Happy to change that if e.g. Guido prefers. :)

    >Well, consenting adults and all. I'm not sure we need to protect ourselves >so strictly against people who don't read the docs and don't understand >Python (i.e. random cargo-culters).

    Python is a popular learning language, and many will be students who haven't yet trained to reflexively look up docs. I saw the lack of such habits in Python's IRC channel.

    "Consenting adults", I feel, is a reason to grant power: don't stop people from doing something they might need to do. But @public on a class method is just an error.

    Would you consider submitting a patch on the project?

    https://gitlab.com/warsaw/public

    fa5ec958-6532-42d7-99e4-e0654bdcb9f7 commented 8 years ago

    I probably won't submit a patch, but I can definitely write a bunch of private notes toward a patch and forget about them. >_>

    Idea 1: Crawl up the call tree and see whether you hit a module definition or a class definition first.

    Idea 2: Inspect __qualname__ to determine whether it's a method/inner function. That might have the same problems with @wraps, though, and you wouldn't be allowed to use a classmethod as a module's function.

    Idea 2 seems to be the right thing. If ".qualname != .name", it's an error. If you really want to use public in the above cases, you should use the assignment form of public (whatever it turns out to be), or explicitly append to all.

    Rule: The programmer should not explicitly use public(a_func), and should use assignment-form public if @public is not possible (== the decorator form is not being used at the point of definition). I think this way won't have errors passing silently and failing mysteriously.

    First draft of error message (not considering assignment-form): "'public' should only be used as a decorator for a function/class in module scope."

    fa5ec958-6532-42d7-99e4-e0654bdcb9f7 commented 8 years ago

    BIKESHEDDING

    Here is another concern with decorators and .__module__ (or inspect.getmodule). (Or is it the original concern?)

    If an earlier decorator creates a wrapper and doesn't set .__module__, you'll make the function public in the wrong module.

        @public
        @bad_wrapper
        def f(...): ...

    IMO, the correct solution is a new stdlib function, to get the currently-loading module. You wouldn't need explicit frame hacking, and it will be portable between different Python implementations because each implementation will have to define it correctly.

    The problem with this solution is the lack of other usecases for such a function, though maybe someone in python-ideas can think of one.

    Candidate owners: sys, importlib.

    Candidate names:

    Effect:

    Other solutions include:

    ----

    I think nonexistence of module.__all__ should be an error.

    The behavior with a module with __all is very different from one without, so I think that if you want your module to have __all, you should be required to create it, even if it's empty.

    Assuming that all is, by convention, always near the top, someone reading your code would know whether the first dozen or so functions are part of the public interface, without searching for @public.

    warsaw commented 8 years ago

    I'm going to go ahead and close this issue. There wasn't much positive reception to @public in the Pycon 2016 language summit (where I presented it as a lightning talk).

    https://gitlab.com/warsaw/public

    Some of the other ideas for changes to Python may still be valid, but they would be best discussed under a new issue.

    zware commented 8 years ago

    I had seen this go by on new-bugs-announce, but hadn't formed an opinion about it. I had also noticed several issues about __all__, but hadn't realized how widespread those issues were. Now I've just been introduced to bpo-23883 via bpo-26809, and was sad to find this closed when I came to give my +1.

    So, +1 anyway. I think this would be rather worthwhile, especially in the stdlib.

    warsaw commented 8 years ago

    On Jun 04, 2016, at 07:47 PM, Zachary Ware wrote:

    So, +1 anyway. I think this would be rather worthwhile, especially in the stdlib.

    Thanks!

    I still like it and plan on continuing to use it in my code. I would recommend you playing with the third party module:

    https://pypi.python.org/pypi/atpublic

    and seeing how you like it. If you do like it maybe help get some momentum behind it. Then we can approach python-dev and try to get it into builtins. I'd be willing to bring it up there (it didn't get such an overwhelming reception at the Pycon 2016 language summit ;).

    ncoghlan commented 7 years ago

    Chiming in so Barry & Zach can take this feedback into account for any future proposal:

    Even then, I'm at best +0 on the proposal, but I also don't make heavy use of code development helpers (IDEs, etc)