Skip to content
This repository has been archived by the owner on Jan 27, 2025. It is now read-only.

Add more Fix functions. #100

Closed
blackwinter opened this issue Dec 22, 2021 · 40 comments · Fixed by #103
Closed

Add more Fix functions. #100

blackwinter opened this issue Dec 22, 2021 · 40 comments · Fixed by #103
Assignees

Comments

@blackwinter
Copy link
Member

blackwinter commented Dec 22, 2021

We're going to need some more Fix functions that aren't implemented yet. E.g.:

Morph Fix
replace replace_all
split split_field
unique uniq

I'm just going to go ahead and implement those plus a bunch more from the Catmandu Cheat Sheet.

Functional review: @TobiasNx
Code review: @fsteeg

@blackwinter blackwinter self-assigned this Dec 22, 2021
@TobiasNx
Copy link
Collaborator

In Catmandu replace_all does not seem to support regexp as in metafacture. That would be nice.

unique seem to process only arrays in Catmandu. Metafacture is here also more powerful.

split_field outputs arrays and differs from split-behavour in Metafacture.

Bonus: Unique/uniq objects not just strings would be awesome.

@blackwinter
Copy link
Member Author

In Catmandu replace_all does not seem to support regexp as in metafacture. That would be nice.

What makes you think that? The cheat sheet says: replace_all(title,'[au]','X') -> title: cXtmXndX. And the tests confirm this behaviour.

unique seem to process only arrays in Catmandu. Metafacture is here also more powerful. [Bonus: Unique/uniq objects not just strings would be awesome.]

Well, yes, it seems that Metamorph's unique function is a little more powerful. But I don't think we've ever made use of that.

split_field outputs arrays and differs from split-behavour in Metafacture.

Hm, other than the absence of arrays in Metamorph, I don't really see a difference. It just emits a repeated field instead of an actual array object.

But thanks for your input! Let's wait and see what our hybrid "better than both worlds" beast will look like in the end ;)

blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
blackwinter added a commit that referenced this issue Jan 11, 2022
@blackwinter blackwinter assigned TobiasNx and unassigned blackwinter Jan 13, 2022
@blackwinter
Copy link
Member Author

@TobiasNx: Sorry, forgot to assign you for functional review.

@TobiasNx
Copy link
Collaborator

This will take some time to review since this are a lot of functions

@blackwinter
Copy link
Member Author

That's understandable ;) Thanks for your diligence!

In effect, this means that #103 and #105 have passed code review, but won't be merged until both functional reviews are done - unless we decide otherwise.

@fsteeg
Copy link
Member

fsteeg commented Jan 17, 2022

In effect, this means that #103 and #105 have passed code review, but won't be merged until both functional reviews are done - unless we decide otherwise.

+1 for merging, we can always open issues/PRs for things that come up in reviews (cc @dr0i for #14).

@blackwinter
Copy link
Member Author

Fine with me, but we should probably not make a habit of it :)

@blackwinter
Copy link
Member Author

If nobody objects I'll merge #103 and #105 tomorrow.

@TobiasNx
Copy link
Collaborator

rename i am not sure anymore if with rename: the sublevels should be change too as default or if that should be an option or done by the each-bind.

TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 24, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 24, 2022
@blackwinter
Copy link
Member Author

random still seems to have some troubles with arrays

These are expected (see #110; requires set_array(...)).

@TobiasNx
Copy link
Collaborator

random still seems to have some troubles with arrays

These are expected (see #110; requires set_array(...)).

you are right, i didn't think about that.

@blackwinter
Copy link
Member Author

blackwinter commented Jan 24, 2022

sort_field: All options do not work yet.

See comment above (use "true").

sort_field: Selecting arrays with * does not work

I'll have to investigate. (see next comment)

@blackwinter
Copy link
Member Author

In general, for inserting/transforming values with wildcards, see #102.

@blackwinter
Copy link
Member Author

reverse("animals[]"): These seem not to work perhapse due to (JSON-)Array. Either way does not work.

Result shows ["zebra", "cat", "dog"], which is the reverse of ["dog", "cat", "zebra"]. See also my previous comment.

@blackwinter
Copy link
Member Author

rename i am not sure anymore if with rename: the sublevels should be change too as default or if that should be an option or done by the each-bind.

Catmandu does it by default (can't be controlled by an option).

TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 24, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 24, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
TobiasNx added a commit to TobiasNx/fix-FunctionalReview-Testing that referenced this issue Jan 25, 2022
@blackwinter
Copy link
Member Author

split_field: This results in empty array:

Without move_field(...) I get the indexed hash:

"OTHERS" : [ {
  "tools" : {
    "1" : "hammer",
    "2" : "saw",
    "3" : "bow"
  }
} ]

So I would ascribe this to our array handling, not the split_field function itself.

Can you confirm?

@TobiasNx
Copy link
Collaborator

Yes that is true and I can confirm that.

I have set up a separate test for moving arrays in object-arrays:

https://github.com/TobiasNx/fix-FunctionalReview-Testing/tree/master/data/testing/move_field_in_object_array

The problem seems to be something if you have an array in an object of an array and if you move this.

@TobiasNx
Copy link
Collaborator

TobiasNx commented Jan 26, 2022

All in all, as discussed off board the function seem to work. All problems that are open are not related to the functions themselves:

  1. Order of elements are changed.
  2. Problems with index-wildcard in paths.

Therefore:
+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants