Hi,

I have this error message ( i.e title ) and i do not see where i am wrong:


    this( in ubyte wordLength, in string sequence ){
        kMer = wordLength;
        bytePerChar = cast(ubyte)(T.sizeof / kMer);

        char[ubyte] toCharTmp;
        ubyte[char] toNumTmp;
        foreach( ubyte i, const char letter; sequence.dup.sort.uniq() ){
            if(i !in toCharTmp)
                toCharTmp[ i ]  = letter;
            toNumTmp[ letter ]  = i;
        }
        toChar  = toCharTmp.rehash;
        toNum   = toNumTmp.rehash;
    }

Error is given at <foreach line>

I have removed explicit type on this line… but I have always same error!

On Wednesday, November 13, 2013 03:43:40 bioinfornatics wrote:
> Hi,
> 
> I have this error message ( i.e title ) and i do not see where i
> am wrong:
> 
> 
>      this( in ubyte wordLength, in string sequence ){
>          kMer = wordLength;
>          bytePerChar = cast(ubyte)(T.sizeof / kMer);
> 
>          char[ubyte] toCharTmp;
>          ubyte[char] toNumTmp;
>          foreach( ubyte i, const char letter;
> sequence.dup.sort.uniq() ){
>              if(i !in toCharTmp)
>                  toCharTmp[ i ]  = letter;
>              toNumTmp[ letter ]  = i;
>          }
>          toChar  = toCharTmp.rehash;
>          toNum   = toNumTmp.rehash;
>      }
> 
> Error is given at <foreach line>
> 
> I have removed explicit type on this line… but I have always same error!

You have two variables to the left to the ; in the foreach. If you were iterating over an array, then the first one would be the index, and the second one would be the element. However, the result of sequence.dup.sort.uniq() is a range that is _not_ an array, and foreach does not support an index for ranges. You can fake it with lockstep if you want, since it does something with tuples to make it so that you get multiple elements on the left-hand side of the semicolon.

http://dlang.org/phobos/std_range.html#lockstep

In addition, I would point out that sequence.dup.sort is using the built-in sort for arrays rather than std.algorithm.sort (you have to have the parens when calling sort on array, or it'll use the built-in one), and the built-in sort for arrays is not only buggy, but it's going to be deprecated, so I wouldn't advise using it. And yes, the means that you'll have to have a random-access range, meaning that you'll need to convert your string to dchar[], but at least then you'll get a sort that works and isn't going to be removed from the language (IIRC, the built-in sort doesn't sort Unicode properly anyway). If you know that you only have ASCII characters, then you can use ubyte[] instead, but char[] isn't going to work, since it's not a random-access range.

- Jonathan M Davis

Jonathan M Davis:

> In addition, I would point out that sequence.dup.sort is using the built-in
> sort for arrays rather than std.algorithm.sort (you have to have the parens
> when calling sort on array, or it'll use the built-in one), and the built-in
> sort for arrays is not only buggy, but it's going to be deprecated, so I
> wouldn't advise using it. And yes, the means that you'll have to have a
> random-access range, meaning that you'll need to convert your string to
> dchar[], but at least then you'll get a sort that works and isn't going to be
> removed from the language (IIRC, the built-in sort doesn't sort Unicode
> properly anyway). If you know that you only have ASCII characters, then you
> can use ubyte[] instead, but char[] isn't going to work, since it's not a random-access range.

I'd like dmd to give a deprecation warning when you use a built-in sort.

And regarding sorting ASCII chars, it's a common need. I usually do it this way (if the input array of chars is mutable the code could spare the dup):

string s = "test";
string t = cast(string)(s.dup.representation.sort().release);

See also:
https://d.puremagic.com/issues/show_bug.cgi?id=10162

Bye,
bearophile

On Wednesday, 13 November 2013 at 02:43:42 UTC, bioinfornatics wrote:
> Hi,
>
> I have this error message ( i.e title ) and i do not see where i am wrong:
>
>
>     this( in ubyte wordLength, in string sequence ){
>         kMer = wordLength;
>         bytePerChar = cast(ubyte)(T.sizeof / kMer);
>
>         char[ubyte] toCharTmp;
>         ubyte[char] toNumTmp;
>         foreach( ubyte i, const char letter; sequence.dup.sort.uniq() ){
>             if(i !in toCharTmp)
>                 toCharTmp[ i ]  = letter;
>             toNumTmp[ letter ]  = i;
>         }
>         toChar  = toCharTmp.rehash;
>         toNum   = toNumTmp.rehash;
>     }
>
> Error is given at <foreach line>
>
> I have removed explicit type on this line… but I have always same error!

Thanks a lot Davis and bearophile that is really interesting I though that I used std.algorithm.sort not a builtin… .

On Wednesday, November 13, 2013 11:02:15 bearophile wrote:
> I'd like dmd to give a deprecation warning when you use a built-in sort.

Agreed, but it's not officially deprecated yet, much as I think that it's supposed to be.

> And regarding sorting ASCII chars, it's a common need. I usually do it this way (if the input array of chars is mutable the code could spare the dup):

Sure, but you can't sort it as char[] that way, and you have to be sure that you're dealing with ASCII-only.

> string s = "test";
> string t = cast(string)(s.dup.representation.sort().release);

I keep forgetting about std.string.representation. I always end up casting (though representation is arguably better).

- Jonathan M Davis