up_to doesnt work correctly when range start is 0

[manyone]

General Summary

the up_to operator doesnt work correctly when range starts at 0.

Steps to Reproduce

1. open the enso IDE
2. enter the graph below (this is the error case - range 0..6 returns a list [0..4] with a length of 5)

3. change the range start to 1 -( correct use - range 1..6 returns a list [1..5] with a length of 5)

[kustosz]

Thank you for the report! Looks like a silly mistake in the definition of to_vector for ranges, on this line: https://github.com/enso-org/enso/blob/main/distribution/std-lib/Base/src/Data/Range.enso#L112 (it always creates a vector of the length equal to end - 1, not the correct expression). A nice low-hanging fruit for potential contributors, wink wink :D

GitHub

enso-org/enso

Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

[wdanilo]

wink wink :D

❤️

[4e6]

Related #1361

[iamrecursion]

As per discussions on discord, up_to should be changed to be inclusive, and until defined to be exclusive of its upper bound. This requires the concept of inclusive and exclusive bounds on ranges to be introduced.

[kustosz]

This requires the concept of inclusive and exclusive bounds on ranges to be introduced.

Does it? Because I see nothing wrong with just defining

Integer.up_to that = Range this that+1
Integer.until that = Range this that

[iamrecursion]

That also works!

[wdanilo]

Integer.up_to that = Range this that+1
Integer.until that = Range this that

I don't like the inconsistency here. We are using the word "up" in the first function but not in the second one. IMO more consistent versions are to + until, or up_to + up_until. The first of course doesn't make sense, as to is "reserved" for our type-mismatch auto conversion.

[kustosz]

W8 so the perceived inconsistency is "not all english expressions start with the same word"?

[wdanilo]

No, but I feel that it is strange that the functions doing almost the same thing use different English constructs. If I know the function up_to and I want a non-inclusive version, I would be searching for up_until or up_to_non_inclusive. I'd be surprised to discover the until name then. I just think that consistency in naming things in API is very important.

[wdanilo]

Ok, as we discussed on Discord, lets use the following API:

range limit by=1 start_inclusive=true end_inclusive=true = ...

up_to limit by=1 start_inclusive=true end_inclusive=true = 
    if this > limit 
        then EptyRange 
        else range limit by start_inclusive end_inclusive

down_to limit by=1 start_inclusive=true end_inclusive=true = 
    if this < limit
        then EmptyRange 
        else range limit by start_inclusive end_inclusive

until limit by=1 start_inclusive=true end_inclusive=false = 
    range limit by start_inclusive end_inclusive

Please note that by doesnt dictate the direction (incrementation vs decrementation), so this expression 10 . range 0 by=2 will produce [10,8,6,4,2]. This way range is the most generic of all forms provided above.

[wdanilo]

I was thinking more about this issue and I think the proposed design (described by me in the previous comment is still wrong).

The problem with inconsistency in the API is very serious and we need to start designing APIs with much bigger caution. It may seem unharmful at the first sight, but it has serious consequences that we would not be able to resolve long-term if we do not put enough care right now. There are few fundamentals for this:

1. Range is a very popular operator (it will even have a special syntax in the language, like 10 .. 0). Thus it's inconsistent naming raised my attention.
2. When designing a graph, it should be easy to design it in a way that works for a broad range of input values. Users should naturally choose components that work for a big range of inputs and should "limit" the inputs fully aware. For example, when creating a component that uses range, it can pass our tests, but users of this component can provide values in a different order (10,0) instead of (0,10). The component should still work correctly. So, for example, yesterday's proposal (on Discord) of range that works only for increasing numbers is bad, error-prone, and allows for accidental creation of non-generic graphs, which completely breaks the UX of the whole product.
3. When adding a component to a stage, its name should clearly tell the user what is its purpose without looking to docs. Otherwise, it is too easy to accidentally create errors that can happen much later, on production.
4. We are doing pilots now. If we use an API in pilot implementations, we would not be able to change it in the future. This is a very important aspect, especially as our stdlib is in so early stage. Thus, we should create a special # unstable doc pragma, which we will add to every function with yet unstable API, so we will not use this function when creating pilots.

Regarding the problem with range, I'd use the following functions to make the naming consistent:

Function name	special syntax	start inclusive by default	end inclusive by default	allows values to increase	allows values to decrease
range	10 .. 0	✅	✅	✅	✅
up_to	none	✅	✅	✅	❌
up_until	none	✅	❌	✅	❌
down_to	none	✅	✅	❌	✅
down_until	none	✅	❌	❌	✅
until	none	✅	❌	✅	✅

Using such naming convention:

• up_ or down_ prefix tells us whether we are using a function that clamps the input. All functions starting with up_ will return an empty range if the start value is bigger than the end value.
• to means inclusive, until means non-inclusive.
• The most generic inclusive function is range, and its non-inclusive counterpart is until. This is the point I don't really like, but I don't have a better solution here. From the purity perspective, range should be named just to, but it is a reserved operator.

Of course, each function above should have optional arguments:

• by:Number - how big is the step (by is positive even if values decrease!)
• start_inclusive:Bool
• end_inclusive:Bool

[iamrecursion]

@wdanilo Please make a decision.

[wdanilo]

@iamrecursion Can we just use what I described above?

[manyone]

can we stick to simply 'range', or  'until'  and allow no special syntax so that range 0..5 represent 0,1,2,3,4,5 and range 1..6 will represent 1,2,3,4,5,6 and both will have the same number of elements (6) and both always start- and end- inclusive.  give a choice to allow or disallow reverse direction.   also allow a stepby option but validate its value at runtime so it agrees with range direction. just my 2 cents. thanks,manny On Thursday, March 25, 2021, 5:13:28 AM PDT, Wojciech Daniło ***@***.***> wrote: @iamrecursion Can we just use what I described above? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[wdanilo]

@manyone Could you elaborate more? Especially, what do you mean by "no special syntax"? We have a syntax of 1 .. 6 - is this the special syntax you are talking about? Also, in my proposal above, there is a function range which is alias for this special sytnax which does exactly what you have written - it is inclusive by default, and works in both ascending and descending orders. There are also aliases like up_to or down_to which can be used if you want to be sure that the input provided for example by the user of your function should iterate in ascending or descending order only. Would you be so nice and provide a table of function that you would implement instead of the table I provided above with a justification why the things I provided are misdesigned for you, please? :)

[manyone]

by special syntax i was referring to the confusing way of coding 0.up_to 7 in order to get a vector of 0..6 what i meant was - why come up with multiple aliases when you can use only one form and tell our users of its behavior, namely - if you use say, 'from_to'  START..END, it will iterate from START to END, inclusive - always ... UNLESS an optional parameter, DIRECTION, is present which indicates +1 means enforce increasing only -1 means enforce decreasing only 0 - any direction allowed.  | from | to | length | direction | result | | 0 | 5 | 6 | 1 | [0,1,2,3,4,5] | | 0 | 5 | 6 | -1 | error | | 0 | 5 | 6 | 0 | [0,1,2,3,4,5] | | 5 | 0 | 6 | 1 | error | | 5 | 0 | 6 | -1 | [5,4,3,2,1,0] | | 5 | 0 | 6 | 0 | [5,4,3,2,1,0] | | 1 | 6 | 6 | 1 | [1,2,3,4,5,6] | | 1 | 6 | 6 | -1 | error | | 1 | 6 | 6 | 0 | [1,2,3,4,5,6] | | 6 | 0 | 6 | 1 | error | | 6 | 0 | 6 | -1 | [6,5,4,3,2,1] | | 6 | 0 | 6 | 0 | [6,5,4,3,2,1] | On Thursday, March 25, 2021, 12:29:14 PM PDT, Wojciech Daniło ***@***.***> wrote: @manyone Could you elaborate more? Especially, what do you mean by "no special syntax"? We have a syntax of 1 .. 6 - is this the special syntax you are talking about? Also, in my proposal above, there is a function range which is alias for this special sytnax which does exactly what you have written - it is inclusive by default, and works in both ascending and descending orders. There are also aliases like up_to or down_to which can be used if you want to be sure that the input provided for example by the user of your function should iterate in ascending or descending order only. Would you be so nice and provide a table of function that you would implement instead of the table I provided above with a justification why the things I provided are misdesigned for you, please? :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[wdanilo]

@manyone, regarding confusing way of coding 0.up_to 7 in order to get a vector of 0..6 - this is a bug in the current stdlib and it is not present anymore in my proposal above. In my proposal, there is up_to function which is marked as inclusive by default on both ends.

In my opinion, there are many cases why we would need to provide several aliases. Sometimes, you want to design a function that will take 2 numbers and will convert that to an ascending range (and only ascending!). So then, you can use up_to. If you want a generic functionality that you have described, in my proposal above there is a range function that behaves exactly as in your table. Would you be so nice and take a look at my table again and check if the proposed functions, especially range suit your needs then?

[manyone]

sorry, i didn't mean to throw a monkey wrench into the process .. as a matter of fact,the range function would be perfect for my needs.  even the  up_to may work.  my only hesitation about using an alias is that the start & end might be different than expected at run time.but otherwise range and up_to will work for me 99.9% of the time. i love enso - i've never had so much fun coding!keep up the good work. thanks,manny On Friday, March 26, 2021, 10:47:33 AM PDT, Wojciech Daniło ***@***.***> wrote: @manyone, regarding confusing way of coding 0.up_to 7 in order to get a vector of 0..6 - this is a bug in the current stdlib and it is not present anymore in my proposal above. In my proposal, there is up_to function which is marked as inclusive by default on both ends. In my opinion, there are many cases why we would need to provide several aliases. Sometimes, you want to design a function that will take 2 numbers and will convert that to an ascending range (and only ascending!). So then, you can use up_to. If you want a generic functionality that you have described, in my proposal above there is a range function that behaves exactly as in your table. Would you be so nice and take a look at my table again and check if the proposed functions, especially range suit your needs then? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[4e6]

Fixed in #1702