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

Add append method to array #3970

Closed
wants to merge 1 commit into from
Closed

Conversation

JustForFun88
Copy link
Contributor

@JustForFun88 JustForFun88 commented Apr 21, 2024

Add append method to array that takes several arrays and creates a new array that sequentially contains all the elements of the provided arrays.

User-facing API

#assert.eq(
  (1, 2).append(("A", "B"), (10, 20)),
  (1, 2, "A", "B", 10, 20),
)

Analogues in other languages

Why is this function needed?

This function is a standard function on lists in almost all programming languages.

I really missed this feature when working with path, that is, when translating vertices from typst format to svg format. It is very inconvenient that in order to connect two arrays of arrays you have to use a for loop. This function, in turn, allows you to ergonomically create a chain of functions, for example like this:

#let res = arr
	.map(x => some_function_with_long_name(x))
	.filter(x => other_function_with_long_name(x))
	.append(..arr2.map(x => x * 2)) // <-- In the middle of the chain!
        .map(x => additional_function(x));

@xkevio
Copy link
Contributor

xkevio commented Apr 21, 2024

That is already possible with the + operator, no? So the usecase here would only be that it's exposed in a method.

#let a = (1, 2, 3)
#let b = ("a", "b", "c")

#{a + b}

image

@JustForFun88
Copy link
Contributor Author

That is already possible with the + operator, no? So the usecase here would only be that it's exposed in a method.

For two arrays, this can indeed be accomplished using the + operator.

@xkevio
Copy link
Contributor

xkevio commented Apr 21, 2024

That is already possible with the + operator, no? So the usecase here would only be that it's exposed in a method.

For two arrays, this can indeed be accomplished using the + operator.

Nope, it works for any number of arrays already.

@JustForFun88
Copy link
Contributor Author

Nope, it works for any number of arrays already.

This operation can only be performed on two arrays.

For example, this works:

#let res = (1, 2) + (3, 4) + (5, 6) + (7, 8);

But this is not:

#let arr = ((3, 4), (5, 6), (7, 8));
#let res = (1, 2) + ..arr;

@JustForFun88
Copy link
Contributor Author

That is already possible with the + operator, no? So the usecase here would only be that it's exposed in a method.

For two arrays, this can indeed be accomplished using the + operator.

Nope, it works for any number of arrays already.

I updated the description, where I described in more detail why this feature is worth adding.

@PgBiel
Copy link
Contributor

PgBiel commented Apr 21, 2024

This is already possible through join(). The code below returns (1, 2, 3, 4):

#((1, 2), (3, 4)).join()

@JustForFun88
Copy link
Contributor Author

JustForFun88 commented Apr 21, 2024

This is already possible through join(). The code below returns (1, 2, 3, 4):

#((1, 2), (3, 4)).join()

Yes, this is indeed possible too. You can also do this:

#let arr = ((3, 4), (5, 6), (7, 8));
#let res = ((1, 2), ..arr);

But what you give are simple examples. However, I think we need to pay attention not only to the fact that append can be replaced with something else, but also to ergonomics. Typst is closer to functional rather than procedural languages, and it also has methods. And therefore it is very convenient to make a chain of functions, for example like this:

#let res = arr
	.map(x => some_function_with_long_name(x))
	.filter(x => other_function_with_long_name(x))
	.append(..arr2.map(x => x * 2)) // <-- In the middle of the chain!
        .map(x => additional_function(x));

In addition, functions similar to join exist in other languages. For example, in the same Gleam there are methods similar to join: concat, flatten. That is, all append, concat, flatten exist side by side. It’s just that some functions are more ergonomic to use in some situations, and others in others.

@laurmaedje
Copy link
Member

One thing that I'm not a big fan of with append is that it sounds like a sequence-alike of push, but here it works differently (returning a new array rather than mutating). That'd be an inconsistency. The mutable operation already exists as += (at least for one array).

Overall, I think sometimes things just break up a chain and I'm not sure whether it's desirable to add new separate ways to write all those cases. Adding the occasional let binding isn't that bad.

@JustForFun88
Copy link
Contributor Author

One thing that I'm not a big fan of with append is that it sounds like a sequence-alike of push, but here it works differently (returning a new array rather than mutating). That'd be an inconsistency. The mutable operation already exists as += (at least for one array).

As I understand correctly, from EcoVec doc, what you say takes place if we have another owner of the vector. If we use this method in a chain of methods (which is usually done), then this method receives a vector by value and returns it. Therefore, this case, i.e. returning a new vector can simply be specified in the documentation.

Overall, I think sometimes things just break up a chain and I'm not sure whether it's desirable to add new separate ways to write all those cases. Adding the occasional let binding isn't that bad.

Well, I don’t know what to say here. I can only point out that all functional languages contain this function and it is one of the key functions, along with map, fold, reduce.

Besides, it's just an array method - one of more than 10 types! Adding this function does not change the syntax of the language and does not introduce new constructs. Analogues of similar functions exist in both ancient languages (hello Lisp) and completely new ones (Gleam). This suggests that this function/method is ergonomic, in demand and time-tested (for more than 60 years).

There are no other arguments left 😅.

@laurmaedje
Copy link
Member

As I understand correctly, from EcoVec doc, what you say takes place if we have another owner of the vector. If we use this method in a chain of methods (which is usually done), then this method receives a vector by value and returns it. Therefore, this case, i.e. returning a new vector can simply be specified in the documentation.

What I meant isn't related to EcoVec, but just to Typst. Consider this:

let x = (1, 2)
let y = x.push(3)
assert.eq(y, none)
assert.eq(x, (1, 2, 3))

If there was an append, I would expect it to work the same way.

let x = (1, 2)
let y = x.append((3, 4))
// is x mutated and y none or is x unchanged and y not none?

This would be an inconsistency people could trip over.

Besides, it's just an array method - one of more than 10 types! Adding this function does not change the syntax of the language and does not introduce new constructs.

I'm not arguing that the problem is the addition of another method, but that this method would introducy inconsistency. Note that in Rust, append is mutable and does not return a new value as in this PR.

Analogues of similar functions exist in both ancient languages (hello Lisp) and completely new ones (Gleam). This suggests that this function/method is ergonomic, in demand and time-tested (for more than 60 years).

What set of methods makes sense and is consistent varies by language. Gleam for instance doesn't seem to have a mutable push operation (from a quick look).

@JustForFun88
Copy link
Contributor Author

What I meant isn't related to EcoVec, but just to Typst. Consider this:

let x = (1, 2)
let y = x.push(3)
assert.eq(y, none)
assert.eq(x, (1, 2, 3))

If there was an append, I would expect it to work the same way.

let x = (1, 2)
let y = x.append((3, 4))
// is x mutated and y none or is x unchanged and y not none?

This would be an inconsistency people could trip over.

How does Typst pass arguments by value? Does Typst always clone it? I expected that it would simply transfer the array itself, and cloning would only occur if we had a second owner. And this can be described in the documentation? Something like that the method is optimized for use in method chains and when applied, it accepts an array, and if there is no second reference to this array, then it returns the same received array, with new elements added. If there is an additional reference to an array, it returns a cloned array with new elements added. It just seems that in Typst you can’t return &mut Array.

@laurmaedje
Copy link
Member

Typst does use clone-on-write internally, but that's not the point I'm trying to make. It is about how the API is used by a user: Typst's push method is mutable while the proposed append is not, which could be surprising to users. I don't really know how else to explain what I mean.

@JustForFun88
Copy link
Contributor Author

Typst does use clone-on-write internally, but that's not the point I'm trying to make. It is about how the API is used by a user: Typst's push method is mutable while the proposed append is not, which could be surprising to users. I don't really know how else to explain what I mean.

I understand you, I just don’t understand why append should be associated with push? After all, users have no problems with map, where it is clearly written “Produces a new array”. Or your words can be used for the filter, rev, join methods. And the sorted method generally fits 100% with your description.

@laurmaedje
Copy link
Member

I think append could be associated with push because it (a) does something quite similar to push and (b) there is no clear consensus among languages whether it is in-place or not. With map and filter on the other hand, people that are familiar with those methods will know immediately that they return something new.

With sorted it's different: It is specifically named that way rather than sort because in some other languages sorted is the one that returns a new array and sort the one that operates in place.

Ultimately this is a very subjective topic, but based on my naming concerns and the discussion and comments from other people, I don't see enough motivation to merge this. Thank you for your PR and effort still!

@laurmaedje laurmaedje closed this May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants