Openrefine: toString() for array arguments in GREL returns a memory address. Consider giving a more intuitive representation.

[lemmingapex]

In GREL, toString( [ 3 ,4]), will return something like: [Ljava.lang.Object;@79738ce6. It would be helpful to provide a more intuitive and natural representation, such as: [ 3, 4 ].
Something like java.util.Arrays.deepToString might give a better meaning when running toString on an array in GREL.

Thoughts?

Some Additional Information:
You will also notice that the expression preview of the array [ 3 ,4] already shows a more natural representation: [ 3, 4 ]. Because arrays are not storable however, attempting to run this transformation leads to the error: Object[] value not storable. This is conflicting behavior. The preview shows a valid transformation, yet the execution of the transformation results in an error. See issue #1088 for more information on storing and displaying arrays in GREL.

[wetneb]

Of course we should have thought about that! Solving this would also fix #2037 in the same go, without introducing a breaking change, so that's clearly the way to go for me.

[thadguidry]

yeah my proof of concept implementation mentioned in #1088 actually used deepToString https://github.com/OpenRefine/OpenRefine/issues/1088#issuecomment-365633202

[darecoder]

@thadguidry @wetneb Please help me understand this issue better like what is exactly supposed to do in this. I'm interested to resolve this issue. Thank you. :smile:

[wetneb]

Have you managed to reproduce the issue on your side? How would you go about working around it? I would be happy to answer specific questions, but I don't think it is useful to describe a step by step solution :)

[darecoder]

@wetneb thanks for the quick response. I have seen this kind of errors in my earlier projects that's why thought try resolving this issue. Steps to reproduce the error was not indicated hence I asked for the help to get better insight. Not intended to bother you a lot.

[thadguidry]

@darecoder1999 Context: Most of our users are not programmers. To them, an array is just a "list with separators". They don't care about Objects, but the fields and data contained within any Object. To get to the data, the GREL convention has historically been toString(). In GREL, we try to provide powerful functions that can be combined together. We typically have tried to avoid overloading functions with too many arguments, and instead create new functions, as necessary. In this case, it could be like that, where rather than overloading or changing toString(), we could certainly introduce a new high level function like say unwrap() or we could go and choose to make it very specific, like arrayToString().

Our philosophy for GREL has been...the best choice for our users will be one that fits with the current workflow style of GREL's existing syntax of functions, and that allows users to extract the data they want, and allow them to format it easily, if necessary inside GREL, by combining with other functions for formatting. (I just realized we don't have a good subsection on our wiki for GREL's originating ideas from David Huynh, the original designer!)

In my example and proof of concept code in that #1088 comment I chose to overload and change the toString() function and I tested it, but not everyone did.

I'd suggest reading through that Google Groups mailing list thread mentioned just above that comment and following along my discussion with @ostephens in the thread at that time.

As well as reading the changes on the toString() function over time on our wiki: https://github.com/OpenRefine/OpenRefine/wiki/GREL-String-Functions#tostringvalue

UPDATE: You'll notice in that wiki page, we say that toString will work with any value type, but that's not 100% true, as in this issue, it doesn't work well with Array types, and we don't currently store Array types in cells yet. :-) My example code fixed this issue in particular, but not tested thoroughly. You are welcome to copy it and provide a PR.

[NavpreetKR]

I am interested to work on this Issue. How can I reproduce this error?

[darecoder]

@wetneb I have reproduced the error and explored grel and expr modules a bit to resolve this issue but not able to find the exact location which is causing this issue. Can you please help me with this, such as in which directory I should look into for this. Any help would be appreciated. Thank you.

[ostephens]

@NavpreetKR @darecoder1999 to reproduce the error in the application start OpenRefine, open a project and in a GREL transformation window (on any column choose dropdown menu and Edit cells -> Transform) and then type the text:

([3,4]).toString()

That will reproduce the error.

In terms of tackling this issue I recommend starting with a test - we have tests for various to/from string conversions in main/tests/server/src/com/google/refine/tests/expr/functions/strings/ToFromConversionTests.java

If you can write some tests to show the correct behaviour then you should be able to see this fail by running the tests. Then start tackling implementing the appropriate behaviour.

Definitely review some of the discussion in #1088 as @thadguidry suggested - I think that will be helpful