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

Symmetry: fn:items-at → fn:get #872

Closed
ChristianGruen opened this issue Dec 5, 2023 · 11 comments
Closed

Symmetry: fn:items-at → fn:get #872

ChristianGruen opened this issue Dec 5, 2023 · 11 comments
Labels
Editorial Minor typos, wording clarifications, example fixes, etc. Propose Closing with No Action The WG should consider closing this issue with no action XQFO An issue related to Functions and Operators

Comments

@ChristianGruen
Copy link
Contributor

I think that fn:items-at should be changed to fn:get:

  • In Standard, array & map functions: Equivalencies #843, we try to harmonize the function names across sequences, maps, and arrays. We have array:get and map:get to retrieve single entries of the input, but we have fn:items-at for sequences.
  • fn:items-at allows you to supply more than a single position, but items-at($seq, (1, 3, 2) can easily be rewritten to (1, 3, 2) ! get($seq, .) – similar to (1, 3, 2) ! array:get($array, .) and (1, 3, 2) ! map:get($map, .).
  • With New sequence functions: names #844, fn:items-at would be the only function left with items in its name.

The function signature would be as simple as:

fn:get(
  $input  as item()*,
  $at     as xs:integer	
) as item()

Obviously, most people will still use $input[$at] – but the same applies to arrays and maps (and other functions like fn:head). One of the advantages of fn:get is that you can pass on the context item as position argument.

@ChristianGruen ChristianGruen added XQFO An issue related to Functions and Operators Editorial Minor typos, wording clarifications, example fixes, etc. Tests Needed Tests need to be written or merged labels Dec 5, 2023
@michaelhkay
Copy link
Contributor

I support the renaming, but I think it's useful to be able to get multiple items. One of the reasons for having this function was that you can't do $seq[1 to 5], but you can do $seq => items-at(1 to 5).

(One possibly unintended benefit of the renaming is that you don't have the problem of using a plural form (items-at) when you are only trying to get one item).

@michaelhkay
Copy link
Contributor

See also issue #825, proposing array:members-at for symmetry with fn:items-at. This would suggest changing array:get to accept multiple integers; but of course, that doesn't work, because if you select more than one member of an array then you want to wrap the result in an array.

@ChristianGruen
Copy link
Contributor Author

See also issue #825, proposing array:members-at for symmetry with fn:items-at. This would suggest changing array:get to accept multiple integers; but of course, that doesn't work, because if you select more than one member of an array then you want to wrap the result in an array.

Exactly. If we want fn:get to be able to return multiple items, and if we place importance on symmetry, we could array:get and map:get let return a flat representation of all values (similar to what *?(3,2,1) does, or array:values and map:values). It would do the job if the values are single items, but it would certainly be unexpected in other cases.

@ChristianGruen
Copy link
Contributor Author

I’ll create a PR for renaming fn:items-at to fn:get, with unchanged semantics. We could open a new issue for…

  • restricting fn:get to return single items, or
  • extending array:get and map:get to return multiple values flattened

@benibela
Copy link

benibela commented Dec 6, 2023

I had written a comment here, and then Github showed a notification, "confirm your 2FA", and then I confirmed it, and now my comment is gone before I could submit it :(

2FA makes everything unusable

@benibela
Copy link

benibela commented Dec 6, 2023

extending array:get and map:get to return multiple values flattened

I had already proposed that in the bug tracker: https://www.w3.org/Bugs/Public/show_bug.cgi?id=29427#c1 (the bug tracker was much better to use, no 2FA there)

And it got rejected for being against "policy".

@michaelhkay
Copy link
Contributor

Sorry about the inconvenience of 2FA, but the vulnerabilities of open source software have become such an issue that it's hardly surprising GitHub are trying to make things more secure.

Your previous 2016 comment was turned down not because it was "against policy", but because it came at the wrong time in the cycle of creating and publishing a spec. You've come up with a lot of good ideas over the years which would probably have made it into the spec if you had been a full participant in the process (knowing when and how to present a proposal that the group will be receptive to is often the key to success). Can I encourage you to join the group? We have a Zoom call for an hour once a week on Tuesdays.

@benibela
Copy link

benibela commented Dec 7, 2023

Your previous 2016 comment was turned down not because it was "against policy",

but you literally wrote "We made a policy decision a long time ago that in the interests of consistency, functions should not implicitly map over their operands. "

but because it came at the wrong time in the cycle of creating and publishing a spec.

but now that there are implementations, one would need to change the spec and the implementations, rather than just the spec, so it is an even worse time to change the function

Can I encourage you to join the group? We have a Zoom call for an hour once a week on Tuesdays.

Tuesdays I have to attend a seminar

@ChristianGruen
Copy link
Contributor Author

@benibela Thanks as well for all your inspiring feedback. Your comments are always interesting.

I had already proposed that in the bug tracker:

Just to be sure (I was not involved in the discussion back then): I suggested here that the second parameter of map:get could be generalized to accept a sequence of keys. It could be useful if the map contains single items as values (and if all requested keys exist in the map). In other cases, the result could be confusing (similar to what $map?($keys) outputs), so I’m not sure whether we should do this.

Did you have the same idea, or didn't you rather propose to change the semantics of function calls on maps?

@ChristianGruen ChristianGruen added the PR Pending A PR has been raised to resolve this issue label Dec 12, 2023
@ChristianGruen
Copy link
Contributor Author

Questions to be answered in the next meeting (or to be dropped, if it takes more than 5–10 minutes):

  1. Accept the PR to replace fn:items-at with fn:get ?
  2. Limit the function to single positions (in alignment with array:get and map:get) ?
  3. If fn:items-at stays unchanged, add array:members-at ?

@ChristianGruen ChristianGruen removed PR Pending A PR has been raised to resolve this issue Tests Needed Tests need to be written or merged labels Jan 19, 2024
@ChristianGruen ChristianGruen added the Propose Closing with No Action The WG should consider closing this issue with no action label Feb 23, 2024
@ndw
Copy link
Contributor

ndw commented Feb 27, 2024

The CG decided to close this issue without further action at meeting 067.

@ndw ndw closed this as completed Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Editorial Minor typos, wording clarifications, example fixes, etc. Propose Closing with No Action The WG should consider closing this issue with no action XQFO An issue related to Functions and Operators
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants