libAtoms / abcd

1 stars 4 forks source link

abcd property manipulation #31

Closed fekad closed 5 years ago

fekad commented 5 years ago

I'm not sure if it's a part of this issue, or separate, but in managing the data I think it will be quite common to want to add a key=value pair (the same one) to a bunch of configurations selected with a query. should this have a separate subcommand? like "abcd tag -q "formula~Fe" newkey=newvalue " or is there a better name ?

Originally posted by @gabor1 in https://github.com/libAtoms/abcd/issues/18#issuecomment-479889973

gabor1 commented 5 years ago

yes, it sounds like a separate subcommand. can always change the name if we come up with something better.

On 29 May 2019, at 17:54, Adam Fekete notifications@github.com wrote:

I'm not sure if it's a part of this issue, or separate, but in managing the data I think it will be quite common to want to add a key=value pair (the same one) to a bunch of configurations selected with a query. should this have a separate subcommand? like "abcd tag -q "formula~Fe" newkey=newvalue " or is there a better name ?

Originally posted by @gabor1 in #18 (comment)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

-- Gábor

Gábor Csányi Professor of Molecular Modelling Engineering Laboratory Pembroke College University of Cambridge

Pembroke College supports CARA. A Lifeline to Academics at Risk. http://www.cara.ngo/

fekad commented 5 years ago

It could be useful to group all the commands of "tag" operation together:

gabor1 commented 5 years ago

This sounds ok, but I would omit the --new. If you find k=v that should be added, the rest can work and I would expect to be able to do multiple operations with a single tag command!

-- Gábor

On 29 May 2019, at 19:52, Adam Fekete notifications@github.com wrote:

It could be useful to group all the commands of "tag" operation together:

Adding new properties abcd tag [-q query_string] --new 'key=value, key2=value2, any_custom_flag' Renaming: abcd tag [-q query_string] --rename force forces Deletion: abcd tag [-q query_string] --delete force — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

gabor1 commented 5 years ago

And so omit the ' getting the new stuff and the commas as well

-- Gábor

On 29 May 2019, at 19:52, Adam Fekete notifications@github.com wrote:

It could be useful to group all the commands of "tag" operation together:

Adding new properties abcd tag [-q query_string] --new 'key=value, key2=value2, any_custom_flag' Renaming: abcd tag [-q query_string] --rename force forces Deletion: abcd tag [-q query_string] --delete force — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

gabor1 commented 5 years ago

I wonder if "tag" is the right name for this. essentially we are talking about modifying the data. adding/deleting/modifying key=value pairs. what do you think ? if we can make this apply to array data seamlessly, it can be merged with another open issue.

fekad commented 5 years ago

Instead of using 'tags' we can use 'property':

abcd property add [-q query] 'key=value key2=value2 any_custom_flag'
abcd property rename [-q query] key new_name
abcd property delete [-q query] key 

I would say the "add" method should work only with keys stored as info. I can't see how can we guarantee that the dimensionality (number of atoms) of the new data must be the same for all of the results of a query.

For modifing properties for arrays, there could be other ways:

1) using a convert command (same as here: https://github.com/libAtoms/abcd/issues/14#issuecomment-497004987)

abcd property convert [-q query] expression

eg.:

abcd property convert [-q query] 'at.info['pressure'] = -1/3 * np.trace(at.info['virial']/at.get_volume())'

2) running a local script (pull -> delete -> convert -> push)

gabor1 commented 5 years ago

I like the "property" subcommand. to run expressions, let's make the subcommand "exec"

gabor1 commented 5 years ago

idea:

instead of a property subcommand, make the sub-sub command just subcommands, so

abcd -q <query> delete-key key
abcd -q <query> rename-key old new
abcd -q <query> add-key key1=value1 key2=value2

if we want to edit an existing key, that should be done via the exec subcommand.

gabor1 commented 5 years ago

does the rename/delete work on array property names too ? (obviously not the add)

fekad commented 5 years ago

Yes, the delete and rename command works on the array property as well. At the moment both of them will be modified. It is not ideal but with the new data structure, hopefully, there won't be more name conflict.

fekad commented 5 years ago

the usage of add-key could cause unexpected behaviour when data with different types has been added. This issue is actually more general so I created a new issue for it #47