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

preparser + match-case = sadness #34678

Open
yyyyx4 opened this issue Oct 20, 2022 · 11 comments
Open

preparser + match-case = sadness #34678

yyyyx4 opened this issue Oct 20, 2022 · 11 comments

Comments

@yyyyx4
Copy link
Member

yyyyx4 commented Oct 20, 2022

sage: x = 2
sage: match x:
....:     case 1: print('hello')
....:     case 2: print('world')
....:
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In [2], line 2
      1 match x:
----> 2     case Integer(1): print('hello')
      3     case Integer(2): print('world')

TypeError: sage.rings.integer.Integer() accepts 0 positional sub-patterns (1 given)

Component: python3

Issue created by migration from https://trac.sagemath.org/ticket/34678

@yyyyx4 yyyyx4 added this to the sage-9.8 milestone Oct 20, 2022
@jhpalmieri
Copy link
Member

comment:1

As a workaround, you can use case 1r: .... I agree that the preparser should be fixed to handle this situation, though.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 20, 2022

comment:2

I think this should be solved by defining __match_args__ for various of our classes, see https://peps.python.org/pep-0634/#class-patterns

@jhpalmieri
Copy link
Member

comment:3

Something like this?

    __match_args__ = ("__match_self_prop__",)

    @property
    def __match_self_prop__(self):
        return self

(taken from that link)

That seems to work for this case, at least.

@jhpalmieri
Copy link
Member

comment:4

How broadly could this be done? For SageObject? Element?

@jhpalmieri
Copy link
Member

comment:5

One option is to modify Element, but I don't know if that's too broad:

diff --git a/src/sage/structure/element.pyx b/src/sage/structure/element.pyx
index b5d83ef71b..11981b1b7c 100644
--- a/src/sage/structure/element.pyx
+++ b/src/sage/structure/element.pyx
@@ -397,6 +397,25 @@ cdef class Element(SageObject):
         """
         self._parent = parent
 
+    # __match_args__ and __match_self_prop__ are to handle Python's
+    # match-case construction. See :trac:`34678` and
+    # https://peps.python.org/pep-0634/#class-patterns.
+    __match_args__ = ("__match_self_prop__",)
+
+    @property
+    def __match_self_prop__(self):
+        """
+        TESTS::
+
+            sage: x = 2
+            sage: match x:
+            ....:     case 1: print('hello')
+            ....:     case 2: print('world')
+            ....:
+            world
+        """
+        return self
+
     def _set_parent(self, parent):
         r"""
         Set the parent of ``self`` to ``parent``.

@jhpalmieri
Copy link
Member

comment:6

The above patch doesn't work, and I think that I really don't understand the match-case construction. With the above patch:

┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 9.8.beta3, Release Date: 2022-10-30               │
│ Using Python 3.10.8. Type "help()" for help.                       │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: x = 3
sage: type(c)
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
Input In [2], in <cell line: 1>()
----> 1 type(c)

NameError: name 'c' is not defined
sage: match x:
....:     case 'b': print('hello')
....:     case c: print('world')
....: 
world
sage: c
3
sage: type(c)
<class 'sage.rings.integer.Integer'>

Why doesn't the match-case block raise an error, since c is not defined? How does c become defined?

@jhpalmieri
Copy link
Member

comment:7

In fact I get this behavior in plain Python. I guess this is binding the undefined symbol c to x and proceeding from there, so this is not a problem with the above patch. However, it still doesn't work:

sage: x = 3
sage: match x:
....:     case 1: print('hello')
....:     case 3.2: print('world')
....: 
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [21], in <cell line: 1>()
      1 match x:
      2     case Integer(1): print('hello')
----> 3     case RealNumber('3.2'): print('world')

TypeError: called match pattern must be a type

It accepts Sage Integers but not RealNumbers.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 15, 2022

comment:8

c becomes defined as a parameter in the match pattern.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 15, 2022

comment:9

The difference is that Integer is a class, but RealNumber is only a function.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 15, 2022

comment:10

I think we would be able to handle RealNumber by changing it to a class with a __classcall__ method.

@jhpalmieri
Copy link
Member

comment:11

That may be more of a job than I have time for right now.

@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants