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

(array|map):replace → *:substitute or *:change #583

Open
michaelhkay opened this issue Jun 29, 2023 · 8 comments · May be fixed by #1833
Open

(array|map):replace → *:substitute or *:change #583

michaelhkay opened this issue Jun 29, 2023 · 8 comments · May be fixed by #1833
Labels
Feature A change that introduces a new feature PR Pending A PR has been raised to resolve this issue PRG-revisit Categorized as "needs revisiting" at the Prague f2f, 2024 XQFO An issue related to Functions and Operators

Comments

@michaelhkay
Copy link
Contributor

Some observations:

  1. array:replace would be more versatile if multiple positions could be specified, rather than just a single position.
  2. if all positions are selected, the function becomes identical to array:for-each. So perhaps we should scrap array:replace and instead add an optional parameter $positions as xs:integer* to array:for-each. However, that could be confusing: people might imagine that the items at positions not present in the list are discarded, rather than being returned unchanged in the result. So I propose we don't do that.
  3. if the function is useful on arrays, then it's also useful on sequences. But fn:replace does something completely different.
  4. We have a similar function on maps called map:substitute (but it's not quite the same, because it processes every entry in the map).

In the interests of alignment, I propose we have three functions:

fn:substitute($input as item()*, $positions as xs:positiveInteger*, $action as function(item()) as item())

equivalent to for $it at $pos in $item return if ($pos = $positions) then $action($it) else $it

array:substitute($array as array(*), $positions as xs:positiveInteger*, $action as function(item()*) as item()*)

equivalent to array{for member $it at $pos in $item return if ($pos = $positions) then $action($it) else $it}

map:substitute($map as map(*), $keys as xs:anyAtomicValue*, $action as function(anyAtomicValue, item()*) as item()*)

For the first two functions, we don't really need to allow the second argument to be omitted, because it would then be equivalent to the corresponding for-each() function. Unfortunately that's not quite true of map:for-each(), because it doesn't return a map. However if you want to do a functional replacement of every entry in a map, it can be done easily enough with

map:build(map:key-value-pairs($map), function{?key}, function{$action(?value)})

so we're not really losing anything.

@ChristianGruen
Copy link
Contributor

ChristianGruen commented Jun 29, 2023

Do we really need the new functions? If position arguments are added to for-each (#516), it will be easy, and much more flexible, to replace specific entries of sequences and arrays. Examples:

for-each($sequence, fn($item, $pos) {
  if($pos > 3) { $item }
})

array:for-each($array, fn($member, $pos) {
  if($pos = (1, 2, 3, 7) or $pos > 10) then $member else $member + 1
})

For maps, either map:for-each or map:put can be used. If we want map:substitute, we should also consider #91.

@ChristianGruen ChristianGruen added XQFO An issue related to Functions and Operators Feature A change that introduces a new feature labels Jun 29, 2023
@michaelhkay
Copy link
Contributor Author

See also issue #553.

@ChristianGruen
Copy link
Contributor

Similar to map:substitute, array:replace seems to have been added to the spec. Should we keep it or drop it?

Related: #104

@ChristianGruen ChristianGruen added the Propose for V4.0 The WG should consider this item critical to 4.0 label Oct 31, 2023
@ChristianGruen ChristianGruen removed the Propose for V4.0 The WG should consider this item critical to 4.0 label Apr 24, 2024
@ChristianGruen
Copy link
Contributor

  1. if the function is useful on arrays, then it's also useful on sequences. But fn:replace does something completely different.

This concern has just been reported back to us by an early adopter, so I would be in favor of dropping map:replace and array:replace as suggested.

Regarding substitutions, fn:change, map:change and array:change could possibly be shorter names.

@ChristianGruen ChristianGruen changed the title array:replace(), etc (array|map):replace → *:substitute or *:change May 22, 2024
@michaelhkay
Copy link
Contributor Author

My current instinct is to drop map:replace and array:replace in favour of the syntax proposed in PR #832

update map $m replace ?price with . * 1.1
update array $a replace ?2 with ()

It has been remarked that we are providing too many solutions to every problem we identify.

@ndw
Copy link
Contributor

ndw commented Jun 4, 2024

We need to revisit this question when we've done the update syntax.

@ndw ndw added the PRG-revisit Categorized as "needs revisiting" at the Prague f2f, 2024 label Jun 4, 2024
@ChristianGruen
Copy link
Contributor

We have no tests for array:replace yet. Do we want to keep the function?

@ChristianGruen
Copy link
Contributor

#1609 proposes revisions for map:replace. Do we want to keep the function or replace it?

@michaelhkay michaelhkay linked a pull request Feb 21, 2025 that will close this issue
@michaelhkay michaelhkay added the PR Pending A PR has been raised to resolve this issue label Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A change that introduces a new feature PR Pending A PR has been raised to resolve this issue PRG-revisit Categorized as "needs revisiting" at the Prague f2f, 2024 XQFO An issue related to Functions and Operators
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants