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

Writing into non-existing arrays with $last, $first and *-wildcard should be skipped? #369

Open
TobiasNx opened this issue Sep 25, 2024 · 6 comments

Comments

@TobiasNx
Copy link
Collaborator

TobiasNx commented Sep 25, 2024

Came up within #368 but is not part of the changes of the related pr this is current master behaviour :

While reading a path with one of the following array wildcards $last, $first and * skips the step due to no existing basis for reading,
writing new values with these array wildcards does not and creates a new element/array with one of wildcard as part of the pathnames

in:
---
a: "Faust" 

copy_field

fix:
copy_field("a","test.*.test")
out:
---
a: "Faust"
test:
  '*':
    test: "Faust"

add_field

fix:
add_field("test.$last.test","a")
out:
---
a: "Faust"
test:
  $last:
    test: "a"

set_array

fix:
set_array("test.$first.test","a")
out:
---
a: "Faust"
test:
  $first:
    test: "a"

In my opinion these creations should be skipped due to non existing arrays and only $append/$prepend should create pathes.

This is connected to: #161

@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Sep 25, 2024

It also seems that catmandu is not that consistend in its behaviour:

https://github.com/TobiasNx/Catmandu-Testing/tree/master/ArrayHandling

$append does not create new array with set_array
$last/$first create new arrays with given values as $append would

*only works it array exists

@fsteeg
Copy link
Member

fsteeg commented Sep 27, 2024

[in catmandu] $last/$first create new arrays with given values as $append would

Then we should do the same here, right?

@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Sep 27, 2024

[in catmandu] $last/$first create new arrays with given values as $append would

Then we should do the same here, right?

To be honest, I am not in favour of this even though Catmandu does this BUT it is not consitent. And Catmandu itself seems not to be consistent with this.

I prefer the behaviour that $last and $first only can do anything if the referred array exists since it is a relational statement imo.

@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Sep 30, 2024

There is a thread concerning the wildcards over at metadaten.community:
https://metadaten.community/t/array-wildcards/456/3

When receiving the answers we can move on.

@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Oct 1, 2024

So the result of my discussion with @jorol at metadaten.community: Catmandus behaviour is:

  • * cannot create a new array
  • $append, $prepend: create new position in an array and can create an array if none exists
    • MF supports this already
  • $first, $last manipulate the referenced position in an array or create an array if none exists or fill up an array if the array is empty THEY DO NOT SKIP array positions if corresponding path exists
    • MF does not support this yet, $last if an empty array exists skips writing if no array exists it writes an object
  • To skip empy arrays one has to use a conditional for copy_field and paste in Catmandu.
    • MF at the moment does that by defaul with $last at the moment
      Exception:
  • all Fix Functions that are of the family set_ (set_field,set_hash, set_array) can not create an array, a position in an array or hash from scratch BUT need the already existing path-structure at least empty. @blackwinter is this related to Change set_field() behaviour to not create intermediate structures. #309 ?
  • You cannot create an empty object or an empty hash in an deep nested way with set_array or set_hash if the structure is already existing.
    • Idea: New Fix function add_array or add_hash could do the trick.

@TobiasNx
Copy link
Collaborator Author

TobiasNx commented Oct 9, 2024

I rephrased the issue.

✔️ MF-Fix supports it
✖️ :MF-Fix does not fully support it

array wildcard refrence reads what? manipulates what? creates an array if non-existing/adds first index-position if array is empty?
* ✔️ all index positions in an array all refrenced matching paths all refrenced matching paths no, creates no new arrays
$prepend ✔️ new position at the beginning of array index cannot read, does not exist yet adds new position to array at the beginning of array index yes
$append ✔️ new position at the end of array index cannot read, does not exist yet adds new position to array at the end of array index yes
$first ✖️ first position in an array index ✔️ reads first position, if exists ✔️ manipulates the first position of an index ✔️ yes ✖️
$last ✖️ last position in an array index ✔️ reads last position, if exists ✔️ manipulates the first position of an index ✔️ yes ✖️

Status:
With #368 $append and $prepend are able to create arrays if they do not exist before.
Before that you had to utilize set_array to create an array first.
$last and first cannot create new arrays at the moment, we also use it intentionally at the moment, to create only something IF the array already exists. (This mechanism could be replaced if the set-fix family would adjusted properly)

In Catmandu the family of the set_-family only add/creates something if the prerequisited/intermediate structure already exists. This is in contrast to all other functions.
=> set_field(test.nested.field, test ) needs rest.nested
=> set_array(test.nested.array.$append) needs test.nested.array
=> set_array(test.nested.array.$last.objectField) needs the hash structuretest.nested.array.$last
=> set_hash(test.nested.inHash.hash) needs test.nested.inhash

Since in context of lobid, rpb and other project we heavily utilize set_array and set_hash to create an intermediate structure, therefore I suggest to rename set_array to add_array and set_hash to add_hash so that we can reuse our transformation without huge changes and that we reimplement

So I suggest that this issue should be fixed together with #309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

3 participants