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.

Sign up for GitHub

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

Jump to bottom

Return namedtuples from tokenize token generator #50107

Closed
mallyvai mannequin opened this issue Apr 27, 2009 · 7 comments
Closed

Return namedtuples from tokenize token generator #50107

mallyvai mannequin opened this issue Apr 27, 2009 · 7 comments
Assignees
@rhettinger
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@mallyvai
Copy link
Mannequin

mallyvai mannequin commented Apr 27, 2009

BPO 5857
Nosy @birkenfeld, @rhettinger, @mallyvai, @superbobry
Files
  • mallyvai_tokenize.patch: Patch to decorate generate_tokens function in tokenize.py and return namedtuples.
  • tokenize.diff: Alternate patch
    		•

    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:

    assignee = 'https://github.com/rhettinger'
closed_at = <Date 2009-04-29.00:35:54.897>
created_at = <Date 2009-04-27.20:26:54.965>
labels = ['type-feature', 'library']
title = 'Return namedtuples from tokenize token generator'
updated_at = <Date 2021-03-11.11:09:22.631>
user = 'https://github.com/mallyvai'

    bugs.python.org fields:

    activity = <Date 2021-03-11.11:09:22.631>
actor = 'superbobry'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2009-04-29.00:35:54.897>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2009-04-27.20:26:54.965>
creator = 'mallyvai'
dependencies = []
files = ['13797', '13799']
hgrepos = []
issue_num = 5857
keywords = ['needs review']
message_count = 7.0
messages = ['86691', '86699', '86701', '86703', '86773', '86774', '388497']
nosy_count = 4.0
nosy_names = ['georg.brandl', 'rhettinger', 'mallyvai', 'superbobry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue5857'
versions = ['Python 3.1', 'Python 2.7']
    @mallyvai
    Copy link
    Mannequin Author

    mallyvai mannequin commented Apr 27, 2009

    Returning an anonymous 5-tuple seems like a suboptimal interface since
    it's so easy to accidentally confuse, for example, the indices of start
    and end. I've used tokenize.py for several scripts in the past few weeks
    and I've always ended up writing some sort of wrapper function for
    generate_tokens that names the returned tuple's fields to help me avoid
    mistakes like this.

    I'd like to propose the following patch that simply decorates the
    generate_token function and names its return values' fields. Since it's
    a namedtuple, it should be fully backwards compatible with the existing
    interface, but also allow member access via

    next_token.type
    next_token.string
    next_token.start.row, next_token.start.col
    next_token.end.row, next_token.end.col
    next_token.line

    If this seems like a reasonable way to do things, I'd be happy to submit
    relevant doc patches as well as the corresponding patch for 3.0.

    @mallyvai mallyvai mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Apr 27, 2009
    @rhettinger
    Copy link
    Contributor

    Comments on the earlier patch:

    • No need for an inner row/col namedtuple. That would add little value.
    • The name "Token" is already used in the module for a different purpose.
    • The wrapper is nice looking, but it is better to in-line this patch.

    Attaching an alternate patch.

    @mallyvai
    Copy link
    Mannequin Author

    mallyvai mannequin commented Apr 27, 2009

    Well, the reason I put in the inner row/col namedtuple initially was
    because the first mistake I made with the original module was mixing up
    the row/col indices for a particular case. It certainly caused all sorts
    of weird headaches. :o)

    I mean, it seems like there's no real reason it "should" be (row,col)
    instead of (col,row) in the returned tuple; that is, it feels like the
    ordering is arbitrary in and of itself.

    I really feel that allowing for start.row and start.col would make the
    interface completely explicit and valid semantically.

    Agreed with the other two points, however.

    Also, I take it there's going to be a need for an addendum to the test
    suite, since the interface is being modified?

    @rhettinger
    Copy link
    Contributor

    I strongly prefer that there not be inner named tuples. That is going
    overboard. FWIW, row/col order is very common convention especially
    when the row refers to a line number in a text block and column refers
    to a character position within the row.

    @rhettinger rhettinger self-assigned this Apr 28, 2009
    @birkenfeld
    Copy link
    Member

    Note that in tokenize.diff, "TokenInfo" should be in __all__ instead of
    "Token". I agree with Raymond on the inner tuples.

    @rhettinger
    Copy link
    Contributor

    Committed in r72086.
    Needs backporting to 2.7.

    @superbobry
    Copy link
    Mannequin

    superbobry mannequin commented Mar 11, 2021

    I strongly prefer that there not be inner named tuples.

    Hi Raymond, do you still strongly prefer (row, col) to remain unnamed? If so, could you comment on what makes you prefer that apart from (row, col) being more common than (col, row)?

    Are there any performance implications/runtime costs associated with making it (row, col) a namedtuple?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @rhettinger rhettinger

    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @birkenfeld @rhettinger