Implemented remaining Python builtins #268

vineetg3 · 2025-02-15T00:19:55Z

Resolves #259

Please check issue for more details.

Few differences from the issues details:

__iter__ : The __iter__ method is not implemented for UnqualifiedNode because doing so would make the node iterable. This behavior could conflict with or disrupt the logic in other components, such as Top_K, which rely on specific handling of non-iterable structures. Implementing __iter__ would introduce unintended behaviors and break the integrity of the system.

The __format__: The __format__ method is not implemented which intentionally does not raise an error because its use interferes with the logic for printing and logging. Additionally, the fallback to __str__ and __repr__ provides sufficiently clear and concise representations, making a custom __format__ unnecessary.

knassre-bodo

Looks great Vineet! Just left a bunch of clarity/nitpick comments to address, then I'll approve.

Also, can you add a new comment to the issue #259 explaining your deviations described in the top of this PR? These are good details to document in the original issue.

knassre-bodo · 2025-02-17T14:45:08Z

documentation/functions.md

+def bad_floor_1():
+   # Using `math.floor` (calls __floor__)
+   return Customer(age=math.floor(order.total_price))


Refactor all of these examples to match the format of other examples in the file (just a single line of code, not a function with a return statement)

knassre-bodo · 2025-02-17T14:45:26Z

documentation/functions.md

@@ -54,6 +54,18 @@ Below is the list of every function/operator currently supported in PyDough as a
 - [Window Functions](#window-functions)
   * [RANKING](#ranking)
   * [PERCENTILE](#percentile)
+- [Unsupported Magic Methods](#unsupported-magic-methods)
+   * [FLOOR](#floor)


Change all of these to the same format as the magic methods: FLOOR -> __floor__

knassre-bodo · 2025-02-17T14:45:59Z

documentation/functions.md

+   * [INDEX](#index)
+   * [LEN](#len)
+   * [CONTAINS](#contains)
+   * [SETITEM](#setitem)


Include any other unsupported ones (__call__ is only for function calls, __bool__ is banned for x and y)

Will we be supporting both .CALCULATE() and conventional syntax of just ()? In that case I'll mention that.

knassre-bodo · 2025-02-17T14:47:39Z

documentation/functions.md

@@ -580,3 +596,164 @@ Customers.WHERE(PERCENTILE(by=acctbal.ASC(), n_buckets=1000) == 1000)
 # For every region, find the top 5% of customers with the highest account balances.
 Regions.nations.customers.WHERE(PERCENTILE(by=acctbal.ASC(), levels=2) > 95)
 ```
+## Unsupported Magic Methods
+
+Below is a list of magic methods that are not supported in PyDough. Calling these methods will result in an Exception.


Magic methods is not a terminology that our users will likely understand, since we are not targeting advanced Python developers. We should simplify this to just be about banned Python logic.

knassre-bodo · 2025-02-17T14:47:46Z

documentation/functions.md

+<!-- TOC --><a name="floor"></a>
+### FLOOR
+
+The `math.floor` function calls the `__floor__` magic method, which is not supported in PyDough.


Not CURRENTLY supported

knassre-bodo · 2025-02-17T14:51:26Z

documentation/functions.md

+<!-- TOC --><a name="contains"></a>
+### CONTAINS
+
+Using the `in` operator calls the `__contains__` magic method, which is not supported in PyDough. This operation is not allowed because the implementation has to return a boolean instead of a PyDough object.


Suggest using ISIN instead

knassre-bodo · 2025-02-17T14:52:30Z

pydough/unqualified/unqualified_node.py

@@ -124,6 +124,10 @@ def __getitem__(self, key):
            args: MutableSequence[UnqualifiedNode] = [self]
            for arg in (key.start, key.stop, key.step):
                coerced_elem = UnqualifiedNode.coerce_to_unqualified(arg)
+                if not isinstance(coerced_elem, UnqualifiedLiteral):
+                    raise PyDoughUnqualifiedException(
+                        "PyDough objects cannot be used as indices in Python slices."


NIT: "are not currently supported"

Are we going to support something like this: Customers[:orders] ?

At the moment, no. Eventually, 🤷?

knassre-bodo · 2025-02-17T14:53:01Z

pydough/unqualified/unqualified_node.py

+        if n is None:
+            raise PyDoughUnqualifiedException(
+                "PyDough requires a specific number of decimal places for rounding."
+            )


Can just change this so the default is 0, like the normal Python behavior.

knassre-bodo · 2025-02-17T14:54:11Z

tests/bad_pydough_functions.py

@@ -73,3 +75,71 @@ def bad_slice_3():
 def bad_slice_4():
    # Unsupported slicing: reversed
    return Customers(name[::-1])
+
+
+def bad_floor_1():


NIT: don't use _1 if it is the only one of its kind.

knassre-bodo · 2025-02-17T14:54:38Z

tests/simple_pydough_functions.py

+def abs_round_magic_method():
+    return DailyPrices(abs_low=abs(low), round_low=round(low, 2))


Also test round(low)

knassre-bodo

Left a few more revision suggestions including some nitpicks, but feel free to merge once you feel they are addressed.

knassre-bodo · 2025-02-19T03:21:16Z

documentation/functions.md

+Using an object in a boolean context calls the `__nonzero__` magic method, which is not supported in PyDough. This operation is not allowed because the implementation has to return an integer instead of a PyDough object.
+
+```py
+Orders(discount=bool(order.total_price))


NIT: lets change this to something where bool makes sense:

Lineitems(is_taxed=bool(tax))

knassre-bodo · 2025-02-19T03:22:00Z

documentation/functions.md

+# Use & instead of and
+Customers.WHERE((acctbal > 0) & (nation.name == "GERMANY"))
+
+# Use | instead of or 
+Orders.WHERE((discount > 0.05) | (tax > 0.08))
+
+# Use ~ instead of not
+Parts.WHERE(~(retail_price > 1000))


Have all of these do the bad thing (and, or, and not, then suggest alternative in the comments)

knassre-bodo · 2025-02-19T03:22:19Z

documentation/functions.md

+The `__bool__` magic method is not supported in PyDough. PyDough code cannot be treated as booleans. Instead of using `or`, `and`, and `not`, use `|`, `&`, and `~` for logical operations. Check out the [Logical](#logical) section for more information.
+
+```py
+# Not allowed - will raise PyDoughUnqualifiedException


Just say this isn't allowed

knassre-bodo · 2025-02-19T03:24:30Z

documentation/functions.md

+The `__call__` magic method is not supported in PyDough, but calling an object will not throw an error. Instead, it internally evaluates to the `CALC` property.
+
+```py
+# These are equivalent:
+Customers(name)
+Customers.__call__(name)
+```


Let's make the examples more consistent & stick with one pattern:

Show a bad behavior with regular Python code (not the magi method)

Include a comment saying it is banned, and why

E.g. for this one it would be:

# Not allowed - calls PyDough code as if it were a function (1 - discount)(extended_price * 0.5)

knassre-bodo · 2025-02-19T03:25:06Z

documentation/functions.md

+The `reversed` function calls the `__reversed__` magic method, which is currently not supported in PyDough.
+
+```py
+Orders(reversed(order_key))


Lets change this to a string so it makes sense, e.g. Regions(backwards_name=reversed(name))

knassre-bodo · 2025-02-19T03:28:13Z

documentation/functions.md

 ### ROUND

-The `ROUND` function rounds its first argument to the precision of its second argument. The rounding rules used depend on the database's round function.
+The `ROUND` function rounds its first argument to the precision of its second argument. The rounding rules used depend on the database's round function. The `round()` magic method is also evaluated to the same. 


Change to "The Python builtin round() function can also be used to accomplish the same thing"

knassre-bodo · 2025-02-19T03:28:34Z

documentation/functions.md

 ### ABS

-The `ABS` function returns the absolute value of its input.
+The `ABS` function returns the absolute value of its input. The `abs()` magic method is also evaluated to the same.


Change to "The Python builtin abs() function can also be used to accomplish the same thing."

knassre-bodo · 2025-02-19T03:29:36Z

documentation/functions.md

+Note: The default precision for `round` magic method is 0, to be in alignment with the Python implementation. The pydough `ROUND` function requires the precision to be specified.
+
+```py
+# This is legal.
+Parts(rounded_price = round(retail_price))
+# This is illegal as precision is not specified.
+Parts(rounded_price = ROUND(retail_price))


Make sure to open a Github issue to fix this behavior.

Done, #276 tackles this.

knassre-bodo · 2025-02-19T03:30:06Z

documentation/functions.md

+Parts(rounded_price = round(retail_price, 1))
+```
+
+Note: The default precision for `round` magic method is 0, to be in alignment with the Python implementation. The pydough `ROUND` function requires the precision to be specified.


NIT: the PyDough ROUND function..

Also, refer to round as a function instead of a magic method.

knassre-bodo · 2025-02-19T03:31:41Z

documentation/functions.md

+
+### \_\_contains\_\_
+
+Using the `in` operator calls the `__contains__` magic method, which is not supported in PyDough. This operation is not allowed because the implementation has to return a boolean instead of a PyDough object. Instead, usage of [ISIN](#isin) function is recommended.


NIT on the last part, change to several recommendations phrased as follows:
"If you need to check if a string is inside another substring, use CONTAINS. If you need to check if an expression is a member of a list of literals, use ISIN."

[RUN CI] implemented magic methods

0a34a79

vineetg3 self-assigned this Feb 15, 2025

vineetg3 requested a review from knassre-bodo February 15, 2025 00:36

[RUN CI] merge conflict

3a331f0

knassre-bodo reviewed Feb 17, 2025

View reviewed changes

[RUN CI] changes

b8db996

knassre-bodo approved these changes Feb 19, 2025

View reviewed changes

vineetg3 added 2 commits February 21, 2025 00:15

[RUN CI]

3cad454

[RUN CI]

edf078d

vineetg3 merged commit b2d548f into main Feb 21, 2025
5 checks passed

vineetg3 deleted the vineet/builtins branch February 21, 2025 21:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented remaining Python builtins #268

Implemented remaining Python builtins #268

vineetg3 commented Feb 15, 2025

knassre-bodo left a comment •

edited

Loading

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

vineetg3 Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

vineetg3 Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo Feb 17, 2025

knassre-bodo left a comment

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

vineetg3 Feb 21, 2025

knassre-bodo Feb 19, 2025

knassre-bodo Feb 19, 2025

		def abs_round_magic_method():
		return DailyPrices(abs_low=abs(low), round_low=round(low, 2))


		### \_\_contains\_\_

		Using the `in` operator calls the `__contains__` magic method, which is not supported in PyDough. This operation is not allowed because the implementation has to return a boolean instead of a PyDough object. Instead, usage of [ISIN](#isin) function is recommended.

Implemented remaining Python builtins #268

Implemented remaining Python builtins #268

Conversation

vineetg3 commented Feb 15, 2025

knassre-bodo left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knassre-bodo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knassre-bodo left a comment •

edited

Loading