Modifying map to allow only key transform #10

TheCelavi · 2025-12-19T08:53:32Z

Allow for map operator to support only key transform.

Can be leveraged for future operators, like leftJoin, or similar.

Copilot

Pull request overview

This PR enhances the map operator to support optional transformations, allowing callers to provide only a key transform, only a value transform, or both. This flexibility enables the operator to preserve original values or keys when only one type of transformation is needed, which can be leveraged for future operators like leftJoin.

Key Changes:

Made both valueTransform and keyTransform parameters optional in the Map operator and map() function
Added validation to ensure at least one transform function is provided
Updated type annotations to reflect the conditional return types based on which transforms are provided

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`src/RunOpenCode/Component/Dataset/src/Operator/Map.php`	Updated constructor to make both transform parameters optional, added validation logic, and updated documentation to reflect the new behavior
`src/RunOpenCode/Component/Dataset/src/functions.php`	Updated `map()` function signature to make `valueTransform` optional and corrected PHPDoc return type annotations
`src/RunOpenCode/Component/Dataset/tests/Operator/MapTest.php`	Renamed existing test and added three new test cases covering key-only, value-only, and validation scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/RunOpenCode/Component/Dataset/src/Operator/Map.php

src/RunOpenCode/Component/Dataset/tests/Operator/MapTest.php

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T09:40:47Z

src/RunOpenCode/Component/Dataset/src/Operator/Map.php

+ * Where transforming function is not provided, original values will be
+ * preserved.


The documentation states "original values will be preserved" but should be more accurate by stating "original values and/or keys will be preserved" since either transformation can be omitted independently. The current wording could be misinterpreted to suggest that only values are preserved when transformations are omitted.

Suggested change

* Where transforming function is not provided, original values will be

* preserved.

* Where transforming function is not provided, original values and/or keys

* will be preserved.

Copilot

Pull request overview

Copilot reviewed 57 out of 57 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T13:08:19Z

docs/source/components/dataset/operators/list/if_empty.rst

+   .. php:method:: __construct(iterable<TKey, TValue> $source, \Throwable|callable(): iterable<TKey, TValue> $fallback)
+
+      :param $source: ``iterable<TKey, TValue>`` Stream source to iterate over.
+      :param $fallback: ``\Throwable|callable(): iterable<TKey, TValue>`` Fallback stream source, or exception to throw.


The documentation signature is missing the nullable type and default value for $fallback. According to the actual IfEmpty operator implementation, the parameter should be \Throwable|callable(): iterable<TKey, TValue>|null $fallback = null since it accepts null and defaults to creating a StreamEmptyException.

Suggested change

.. php:method:: __construct(iterable<TKey, TValue> $source, \Throwable|callable(): iterable<TKey, TValue> $fallback)

:param $source: ``iterable<TKey, TValue>`` Stream source to iterate over.

:param $fallback: ``\Throwable|callable(): iterable<TKey, TValue>`` Fallback stream source, or exception to throw.

.. php:method:: __construct(iterable<TKey, TValue> $source, \Throwable|callable(): iterable<TKey, TValue>|null $fallback = null)

:param $source: ``iterable<TKey, TValue>`` Stream source to iterate over.

:param $fallback: ``\Throwable|callable(): iterable<TKey, TValue>|null`` Fallback stream source, exception to throw, or ``null`` to use the default empty-stream exception.

Copilot · 2025-12-23T13:08:20Z

docs/source/components/dataset/operators/list/map.rst

+
+      :param $source: ``iterable<TKey, TValue>`` Stream source to iterate over.
+      :param $valueTransform: ``?callable(TValue, TKey=): TModifiedValue`` Optional transformation function for transforming values.
+      :param $keyTransform: ``?callable(TKey, TValue=): TModifiedKey`` Optional transformation function for transforming keys.


The documentation is missing the parameter name for $keyTransform in the :param directive. It should be :param $keyTransform: instead of just starting with the type.

Copilot · 2025-12-23T13:08:20Z

docs/source/components/dataset/operators/list/map.rst

+=====
+
+Map operator iterates over given stream source and applies transformation
+functions one keys/values before yielding.


There's a typo in "functions one keys/values" which should be "functions on keys/values".

Suggested change

functions one keys/values before yielding.

functions on keys/values before yielding.

Copilot · 2025-12-23T13:08:20Z

docs/source/components/dataset/operators/list/map.rst

+       ->map(function(array $row): Entity {
+           return Entity::fromArray($row);
+       }, function(int $key, array $row): string {
+           return $row['id'];
+       });


The example uses positional arguments but doesn't make the intent clear. The first callable transforms values (array -> Entity) and the second transforms keys (extracting 'id' from the row). To improve clarity and match the pattern used in the test files, this example should use named parameters: valueTransform: function(array $row): Entity { ... }, keyTransform: function(int $key, array $row): string { ... }.

Suggested change

->map(function(array $row): Entity {

return Entity::fromArray($row);

}, function(int $key, array $row): string {

return $row['id'];

});

->map(

valueTransform: function(array $row): Entity {

return Entity::fromArray($row);

},

keyTransform: function(int $key, array $row): string {

return $row['id'];

},

);

Copilot · 2025-12-23T13:08:21Z

docs/source/components/dataset/operators/list/distinct.rst

+   .. php:method:: __construct(iterable<TKey, TValue> $source, ?callable(TValue, TKey=): string $identity = null)
+
+      :param $source: ``iterable<TKey, TValue>`` Stream to iterate over.
+      :param $predicate: ``?callable(TValue, TKey=): string`` User defined callable to determine item identity. If null, strict comparison (===) of values is used.


The parameter name in the documentation is incorrect. The method signature shows the parameter is named $identity, but the parameter description refers to it as $predicate.

Suggested change

:param $predicate: ``?callable(TValue, TKey=): string`` User defined callable to determine item identity. If null, strict comparison (===) of values is used.

:param $identity: ``?callable(TValue, TKey=): string`` User defined callable to determine item identity. If null, strict comparison (===) of values is used.

Copilot · 2025-12-23T13:08:21Z

docs/source/components/dataset/operators/list/filter.rst

+
+.. php:class:: Filter
+
+   .. php:method:: __construct(iterable<TKey, TValue> $source, callable(TValue, TKey=): bool $filter = null)


The method signature shows a default value of null for the $filter parameter, but the actual Filter operator constructor does not have a default value for this parameter (it's required). The documentation should remove = null from the signature.

Suggested change

.. php:method:: __construct(iterable<TKey, TValue> $source, callable(TValue, TKey=): bool $filter = null)

.. php:method:: __construct(iterable<TKey, TValue> $source, callable(TValue, TKey=): bool $filter)

Copilot · 2025-12-23T13:08:21Z

src/RunOpenCode/Component/Dataset/src/Operator/Finalize.php

+        $this->finalized = true;
+
        try {
            yield from $this;


This creates infinite recursion. The iterate() method yields from $this, which calls getIterator(), which calls iterate() again. This should yield from the source stream instead: yield from $this->source;

Copilot · 2025-12-23T13:08:21Z

src/RunOpenCode/Component/Dataset/src/Operator/Map.php

 *
- * Map operator iterates over given collection and yields transformed items.
+ * Map operator iterates over given stream source and applies transformation
+ * functions one keys/values before yielding.


There's a typo in "functions one keys/values" which should be "functions on keys/values".

Copilot · 2025-12-23T13:08:22Z

src/RunOpenCode/Component/Dataset/src/Operator/IfEmpty.php

-        $this->action = $action instanceof \Exception ? static fn(): never => throw $action : $action(...);
+        parent::__construct($source);
+        $fallback       = $fallback ?? new StreamEmptyException();
+        $this->fallback = $fallback instanceof \Throwable ? static fn(): \Throwable => throw $fallback : $fallback(...);


The closure return type annotation is incorrect. When $fallback is a \Throwable, the closure should be declared as static fn(): never => throw $fallback (not static fn(): \Throwable), since throwing always results in never returning. The current type suggests the closure returns an exception object, which is not what happens.

TheCelavi requested review from Copilot and stefan1144 December 19, 2025 08:53

Copilot started reviewing on behalf of TheCelavi December 19, 2025 08:53 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

src/RunOpenCode/Component/Dataset/src/Operator/Map.php Outdated Show resolved Hide resolved

src/RunOpenCode/Component/Dataset/tests/Operator/MapTest.php Outdated Show resolved Hide resolved

TheCelavi force-pushed the dev branch from 7684683 to 0ee7a5e Compare December 19, 2025 09:32

Copilot AI review requested due to automatic review settings December 19, 2025 09:37

Modifying map to allow only key transform

aa897a4

TheCelavi force-pushed the dev branch from 4e6a4ea to aa897a4 Compare December 19, 2025 09:37

Copilot started reviewing on behalf of TheCelavi December 19, 2025 09:37 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

TheCelavi added 2 commits December 19, 2025 13:26

Adding some more documentation

012dca6

Making dataset API stabile

524f678

Copilot AI review requested due to automatic review settings December 23, 2025 13:01

Copilot started reviewing on behalf of TheCelavi December 23, 2025 13:01 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

stefan1144 approved these changes Dec 23, 2025

View reviewed changes

Making dataset API stabile

42c4990

TheCelavi merged commit ec332aa into master Dec 23, 2025
1 check passed

		* Where transforming function is not provided, original values will be
		* preserved.

	functions one keys/values before yielding.
	functions on keys/values before yielding.

	:param $predicate: ``?callable(TValue, TKey=): string`` User defined callable to determine item identity. If null, strict comparison (===) of values is used.
	:param $identity: ``?callable(TValue, TKey=): string`` User defined callable to determine item identity. If null, strict comparison (===) of values is used.


		.. php:class:: Filter

		.. php:method:: __construct(iterable<TKey, TValue> $source, callable(TValue, TKey=): bool $filter = null)

Modifying map to allow only key transform #10

Modifying map to allow only key transform #10

Uh oh!

Conversation

TheCelavi commented Dec 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants