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

Fix cross function example #218

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Fix cross function example #218

merged 1 commit into from
Apr 27, 2023

Conversation

wetneb
Copy link
Member

@wetneb wetneb commented Apr 26, 2023

It looks like the only example of the cross function we had in the docs was actually wrong… Can anyone confirm?

It looks like the only example of the `cross` function we had in the docs was actually wrong… Can anyone confirm?
@wetneb wetneb requested a review from ostephens April 26, 2023 17:40
@netlify
Copy link

netlify bot commented Apr 26, 2023

Deploy Preview for openrefine-website ready!

Name Link
🔨 Latest commit e39801a
🔍 Latest deploy log https://app.netlify.com/sites/openrefine-website/deploys/6449620b0d050d0008686f15
😎 Deploy Preview https://deploy-preview-218--openrefine-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@thadguidry
Copy link
Member

thadguidry commented Apr 27, 2023

Yes, the example is wrong in the sense that an Array might be the result, or it might not. Depends on if it returns an [ [ object Row ] ] Array technically? For now that's not likely to change? Until we introduce storing Arrays in cells?

The original expression syntax was to get the 1st Row from the Array
cell.cross("TestCross","Name")[0] et.al. @dfhuynh
But dunno if there is always 1 Row or Record only now after @antoine2711 's changes...which is slightly concerning, but we'll figure it out.

image
image
image
image
image
image

@antoine2711
Copy link
Member

@wetneb : yes, the documentation is wrong.

cell.cross("People","Name").cells["Address"].value[0]

should of course be

cell.cross("People","Name")[0].cells["Address"].value

Regards, A.

@antoine2711 antoine2711 merged commit 648b284 into master Apr 27, 2023
@antoine2711
Copy link
Member

But dunno if there is always 1 Row or Record only now after @antoine2711 's changes... which is slightly concerning, but we'll figure it out.

No, this has not changed with the fix 3 years ago. Somehow, the doc got broken. Sad, but good thing it's now fixed.

Regards,
Antoine

@ostephens
Copy link
Member

In fact both expressions work. Although cross (as it has always done afaik) returns an array of rows, it seems that .cells[] will iterate of the array of rows and and value will iterate over the array of rows. So:

cell.cross("People","Name") -> array of row objects
cell.cross("People","Name").cells["Address"] -> array of cell objects
cell.cross("People","Name").cells["Address"].value -> array of values

So
cell.cross("People","Name").cells["Address"].value[0]
and
cell.cross("People","Name").cells["Address"][0].value
and
cell.cross("People","Name")[0].cells["Address"].value

all give the same result

I'm not sure if this has always been the case, but I always use (and teach) the last of these as it makes most sense to me, but I guess the other options provide some convenience if you just need a single value

@wetneb
Copy link
Member Author

wetneb commented Apr 27, 2023

Ah, good point. Actually, I find it really messy that GREL silently applies a computation to the elements of an array as a map.

@tfmorris
Copy link
Member

All of these examples deemphasize the fact that cross() can return multiple rows. I think it would be better for the example to using join() to highlight handling of multiple values.

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.

5 participants