search

LibreCat / Catmandu

Catmandu - a data processing toolkit
https://librecat.org
175 stars 31 forks source link

introduce a new fix map to move several fields by a lookup table #366

Closed vpeil closed 2 years ago

vpeil commented 5 years ago

in analogy the the lookup fix: this one lookups up in the table and moves the fields instead of changing the values.

vpeil commented 5 years ago

this one solves #67

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.01%) to 89.256% when pulling ef75f189e97091fdc2ef2988deebdba92ea467d7 on pr/new-fix-map into 7c4c05dbb851f53e795620ed7325232c8afee8bd on dev.

nichtich commented 3 years ago

I'd like to rename this to mapping because map more likely refers to an individual row. We have fixes marc_map and pica_map, these could be extended with fixes marc_mapping and pica_mapping similar to this new fix. ok?

vpeil commented 3 years ago

mapping is indeed better, looks good otherwise

name of fix changed from map to mapping

vpeil commented 3 years ago

The tests fail for perl version 5.10 and 5.12, because one dependency module requires at least perl 5.14. Should we move on from these two versions?

nichtich commented 3 years ago

The failure was not caused by this PR but by https://github.com/LibreCat/Catmandu/issues/379

nichtich commented 3 years ago

I'd make deletion optional with option delete: 0 (default: 1) so you can also copy instead of rename.

vpeil commented 3 years ago

I'd make deletion optional with option delete: 0 (default: 1) so you can also copy instead of rename.

@nichtich delete option could be misinterpreted by users: it could mean "delete all fields not in the mapping table" (as we use it in the lookup fix). Maybe find another option name?

nichtich commented 3 years ago

How about keep to not delete anything?

vpeil commented 2 years ago

@nics @nichtich Anything else that prevents it from a merge?