Skip to content

symtable.Class.get_methods() returns non-methods #119698

Closed

Description

@JelleZijlstra

Bug report

Bug description:

Running this file:

import symtable

st = symtable.symtable("""
class X:
    class Nested: pass
    type Alias = int
    x = (x for x in range(10))
    y = filter(lambda z: z % 2, range(10))
""", "mod", "exec")

cls = st.get_children()[0]
print(cls.get_methods())

Prints:

('Nested', 'Alias', 'genexpr', 'lambda')

None of these are methods.

@carljm and I noticed this as part of the work on PEP 649 (#119361 (comment)). My draft implementation excludes generated __annotate__ functions from the list of "methods", but perhaps all non-methods should be excluded.

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

macOS

Linked PRs

Activity

  1. changed the title [-]symtable.Class.get_methods() returns non-classes[/-] [+]symtable.Class.get_methods() returns non-methods[/+] on May 29, 2024
  2. serhiy-storchaka commented on May 29, 2024

    @serhiy-storchaka
    Member

    The symtable CLI

    $ echo "
    class X:
        class Nested: pass
        type Alias = int
        x = (x for x in range(10))
        y = filter(lambda z: z % 2, range(10))
    " | ./python -m symtable
    

    shows details:

    symbol table for module from file '<stdin>':
        local symbol 'X': def_local
    
        symbol table for class 'X':
            local symbol 'Nested': def_local
            local symbol 'Alias': def_local
            local symbol 'x': def_local
            global_implicit symbol 'range': use
            local symbol 'y': def_local
            global_implicit symbol 'filter': use
    
            symbol table for class 'Nested':
    
            symbol table for type alias 'Alias':
                free symbol '__classdict__': use
                global_implicit symbol 'int': use
    
            symbol table for function 'genexpr':
                local symbol '.0': def_param
                local symbol 'x': use, def_local
    
            symbol table for function 'lambda':
                local symbol 'z': use, def_param
    

    We see that get_methods() returns the names of nested symbol tables. Some symbol table names like "genexpr" and "lambda" has no relation to the name of any nested symbol. It is a question why get_methods() can be useful at all.

    We can simply deprecate get_methods(). Or define it as returning names of local symbols that has corresponding symbol table with type "function". Then it may be worth to add also get_classes() and get_aliases(). What do you need?

  3. JelleZijlstra commented on May 29, 2024

    @JelleZijlstra
    MemberAuthor

    Then it may be worth to add also get_classes() and get_aliases(). What do you need?

    I have never used this module and I don't know how people are using it; I ran into this problem because some tests failed while I was working on the PEP 649 implementation.

    Deprecating this method seems like a reasonable path forward.

  4. JelleZijlstra commented on Jun 4, 2024

    @JelleZijlstra
    MemberAuthor

    cc @picnixz in case you're interested in other problems with the symtable module.

  5. picnixz commented on Jun 4, 2024

    @picnixz
    Member

    Personally, I've never used it but I was interested in contributing to CPython so I started where I thought it was something not too hard and interesting. One use case that I have in mind is IntelliSense but I would expect those tools to actually use the AST instead (but since their code is private, I cannot say for sure).

    If there is really no use-case, I'd suggest deprecating it (less things to maintain, easier for us). I've tried to search whether it's used or not on github but I've only seen this occurrence https://github.com/skulpt/skulpt/blob/8daae6db0d2834ae2e66f6407e00239d4a95ead1/skulpt.py#L101 being relevant (others are not I think). However, they are using the thing for Python 2 and are not claiming full Python 3 compatibility.

  6. picnixz commented on Jun 6, 2024

    @picnixz
    Member

    I've implemented two suggestions so feel free to pick either of the two.

    I decided to keep the PR fixing the interface since Carl and Jelle have reviewed it and the work was not that much.

  7. picnixz commented on Jun 12, 2024

    @picnixz
    Member

    For the record, I've implemented get_aliases and get_classes using a more generic approach in a separate branch: https://github.com/picnixz/cpython/tree/feat-symtable-classes

    Note that the branch actually contains a local copy of #120151 (and I will not open a PR yet).

  8. JelleZijlstra commented on Jun 12, 2024

    @JelleZijlstra
    MemberAuthor

    I would only add such methods if someone comes up with an actual use case.

  9. serhiy-storchaka commented on Jun 12, 2024

    @serhiy-storchaka
    Member

    I only have found the use of this method in the skulpt project, which is Python 2 only. I also prefer to deprecate it.

  10. picnixz commented on Jun 12, 2024

    @picnixz
    Member

    To summarize, we would fix the method in 3.14 (and 3.12, 3.13, depending on the backport decision), deprecate it in 3.15 and remove it in 3.17? (and it's fine that we don't need the other methods)

  11. serhiy-storchaka commented on Jun 12, 2024

    @serhiy-storchaka
    Member

    Why not deprecate it in 3.14?

  12. picnixz commented on Jun 12, 2024

    @picnixz
    Member

    Err..., maybe someone will be interested in using it since we fixed it? (I can add the deprecation in the same or a follow-up PR though)

  13. JelleZijlstra commented on Jun 12, 2024

    @JelleZijlstra
    MemberAuthor

    We should definitely do only one of deprecating and fixing, not both; it doesn't make sense to radically change the implementation of the method and then immediately deprecate it.

    I would prefer to fix the method instead of deprecating it though. We have a solution that fixes the method (#120151), and I don't think maintaining the method in the future will be very costly. I think we should only deprecate the method if there's some strong reason we don't want it in the library in the future.

  14. serhiy-storchaka commented on Jun 12, 2024

    @serhiy-storchaka
    Member

    If we are going to fix get_methods(), we should also add get_classes(), get_aliases(), etc in the Class class. Then add the SymbolTable subclass Module with methods get_classes(), get_functions(), etc. Then perhaps add get_classes() and get_functions() also in the Function class, for local classes and functions.

    Note that currently functions which return a limited set of names, such as get_parameters() and get_locals(), only filter names of symbols by their scope and flags. New get_methods(), get_classes(), etc, will be more complex, they need to look into two tables: symbols and symboltables. Too complex behavior for a feature that no one will possibly use.

  15. 16 remaining items

  16. added 2 commits that reference this issue on Jul 11, 2024
  17. picnixz commented on Jul 17, 2024

    @picnixz
    Member

    Re-opening until the fix of the fix (+backports) lands and the deprecation notice is merged.

  18. added 2 commits that reference this issue on Jul 17, 2024
  19. added a commit that references this issue on Jul 17, 2024
  20. added 2 commits that reference this issue on Jul 17, 2024
  21. added a commit that references this issue on Jul 17, 2024
  22. serhiy-storchaka commented on Jul 17, 2024

    @serhiy-storchaka
    Member

    I opened also #121914.

  23. added 2 commits that reference this issue on Jul 17, 2024
  24. God-damnit-all commented on Jun 8, 2025

    @God-damnit-all

    This method looks extremely useful (my own attempts to make something similar were met wth countless problems) and I only found out about this because it's now deprecated. How unfortunate.

    If we are going to fix get_methods(), we should also add get_classes(), get_aliases(), etc in the Class class. Then add the SymbolTable subclass Module with methods get_classes(), get_functions(), etc. Then perhaps add get_classes() and get_functions() also in the Function class, for local classes and functions.

    Note that currently functions which return a limited set of names, such as get_parameters() and get_locals(), only filter names of symbols by their scope and flags. New get_methods(), get_classes(), etc, will be more complex, they need to look into two tables: symbols and symboltables. Too complex behavior for a feature that no one will possibly use.

    These would all be invaluable to me.

    It's probable that no one ever recommended this to me in the past because it was broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions