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.

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

Implement matrix multiplication operator (PEP 465) #65375

Closed
abalkin opened this issue Apr 8, 2014 · 15 comments
Closed

Implement matrix multiplication operator (PEP 465) #65375

abalkin opened this issue Apr 8, 2014 · 15 comments
Assignees
Labels
interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@abalkin
Copy link
Member

abalkin commented Apr 8, 2014

BPO 21176
Nosy @jcea, @abalkin, @benjaminp, @njsmith
Files
  • mat-mult1.patch
  • mat-mult2.patch
  • mat-mult3.patch
  • mat-mult4.patch
  • mat-mult5.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/benjaminp'
    closed_at = <Date 2014-04-10.03:56:05.754>
    created_at = <Date 2014-04-08.02:51:34.953>
    labels = ['interpreter-core', 'type-feature']
    title = 'Implement matrix multiplication operator (PEP 465)'
    updated_at = <Date 2014-06-12.00:57:19.493>
    user = 'https://github.com/abalkin'

    bugs.python.org fields:

    activity = <Date 2014-06-12.00:57:19.493>
    actor = 'jcea'
    assignee = 'benjamin.peterson'
    closed = True
    closed_date = <Date 2014-04-10.03:56:05.754>
    closer = 'python-dev'
    components = ['Interpreter Core']
    creation = <Date 2014-04-08.02:51:34.953>
    creator = 'belopolsky'
    dependencies = []
    files = ['34754', '34755', '34756', '34758', '34762']
    hgrepos = []
    issue_num = 21176
    keywords = ['patch']
    message_count = 15.0
    messages = ['215724', '215726', '215730', '215731', '215732', '215734', '215735', '215736', '215737', '215738', '215739', '215740', '215762', '215863', '215865']
    nosy_count = 5.0
    nosy_names = ['jcea', 'belopolsky', 'benjamin.peterson', 'njs', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21176'
    versions = ['Python 3.5']

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 8, 2014

    [Nathaniel Smith at numpy-discussion]

    Guido just formally accepted PEP-465:
    https://mail.python.org/pipermail/python-dev/2014-April/133819.html
    http://legacy.python.org/dev/peps/pep-0465/#implementation-details

    Yay.

    The next step is to implement it, in CPython and in numpy. I have time
    to advise on this, but not to do it myself, so, any volunteers? Ever
    wanted to hack on the interpreter itself, with BDFL guarantee your
    patch will be accepted (if correct)?

    The todo list for CPython is here:
    http://legacy.python.org/dev/peps/pep-0465/#implementation-details
    There's one open question which is where the type slots should be
    added. I'd just add them to PyNumberMethods and then if someone
    objects during patch review it can be changed.

    @abalkin abalkin self-assigned this Apr 8, 2014
    @abalkin abalkin added interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Apr 8, 2014
    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    Here's a first patch. It works, but needs more tests (associativity, precedence, __rmatmul__, etc...) and also docs. I can work on that tomorrow.

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    Here are changes to the operator module...

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 8, 2014

    I've got to the point where I can do

    >>> import ast
    >>> ast.dump(ast.parse('a @ b'))
    "Module(body=[Expr(value=BinOp(left=Name(id='a', ctx=Load()), op=MatMult(), right=Name(id='b', ctx=Load())))])"

    I'll post a link to my bitbucket clone shortly.

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    Here is a nearly complete patch...

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 8, 2014

    Wow! That was quick. And I am still fighting with bitbucket. Maybe I should stop duplicating the effort at this point.

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    On Mon, Apr 7, 2014, at 22:09, Alexander Belopolsky wrote:

    Alexander Belopolsky added the comment:

    Wow! That was quick. And I am still fighting with bitbucket. Maybe I
    should stop duplicating the effort at this point.

    Sorry about that. I only saw the first message, where Nathaniel invites
    an implementation. I didn't realize that was a direct quote from the
    mailing list (not by you) and didn't notice you also assigned it to
    yourself.

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 8, 2014

    Thanks for stepping in. From a quick look at your patch I don't see anything that I would do much differently.

    I noticed some whitespace issues in Include/token.h:

    -#define AT              49	
    -#define RARROW          50
    -#define ELLIPSIS        51
    +#define AT              49
    +#define ATEQUAL		50
    +#define RARROW          51
    +#define ELLIPSIS        52

    (Are you sure you want to re-enumerate RARROW and ELLIPSIS? Why not just give ATEQUAL the value of 52?)

    @abalkin
    Copy link
    Member Author

    abalkin commented Apr 8, 2014

    + .. versionadded:: 3.4

    Are you planning to use the time machine? :-)

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    On Mon, Apr 7, 2014, at 22:23, Alexander Belopolsky wrote:

    Alexander Belopolsky added the comment:

    Thanks for stepping in. From a quick look at your patch I don't see
    anything that I would do much differently.

    I noticed some whitespace issues in Include/token.h:

    -#define AT 49
    -#define RARROW 50
    -#define ELLIPSIS 51
    +#define AT 49
    +#define ATEQUAL 50
    +#define RARROW 51
    +#define ELLIPSIS 52

    (Are you sure you want to re-enumerate RARROW and ELLIPSIS? Why not just
    give ATEQUAL the value of 52?)

    I thought it would be nicer if ATEQUAL was right after AT.

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    On Mon, Apr 7, 2014, at 22:25, Alexander Belopolsky wrote:

    Alexander Belopolsky added the comment:

    • .. versionadded:: 3.4

    Are you planning to use the time machine? :-)

    Good catch. :)

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    Here's my latest and greatest version. I'll finish up after some sleep.

    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 8, 2014

    Here's what I consider a complete patch.

    @benjaminp benjaminp assigned benjaminp and unassigned abalkin Apr 8, 2014
    @benjaminp
    Copy link
    Contributor

    benjaminp commented Apr 10, 2014

    I think this patch is fairly straightforward, and I don't want it to rot, so here we go...

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 10, 2014

    New changeset c553d8f72d65 by Benjamin Peterson in branch 'default':
    PEP-465: a dedicated infix operator for matrix multiplication (closes bpo-21176)
    http://hg.python.org/cpython/rev/c553d8f72d65

    @python-dev python-dev mannequin closed this as completed Apr 10, 2014
    @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
    Labels
    interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants