Append and readProperties

⁨0⁩ ⁨likes⁩

Submitted ⁨⁨2⁩ ⁨months⁩ ago⁩ by ⁨Plugin Development@discuss.avogadro.cc [bot]⁩ to ⁨Plugin Development@discuss.avogadro.cc⁩

One more thing to do with the plugin interface that I'd like to ask questions about before release. It might just be a case of clarifying things so that I can write the documentation accurately; it might mean a small last-minute adjustment to how Avogadro handles things.

Plugins are currently able to pass back to Avogadro, as part of the output JSON object, two key/value pairs that should have a similar effect:

"append": true
"readProperties": true

Both are by default false if not specified.

The default behaviour of Avogadro for a menuCommand is to replace the entire contents of the current file with whatever the plugin sends back. If the file/geometry that the plugin sends back is not a CJSON, Avogadro first converts the returned format into CJSON. Thus if a plugin sends nothing back, the current molecule/file disappears. For this reason, some commands in the xtb plugin take care to send back the entire input CJSON with zero changes, because the commands only do something like open a URL in the web browser.

I believe both append and readProperties are meant to indicate to Avogadro something like "don't discard the current file, don't replace it with what I am giving you, just supplement the current file with whatever I am giving you".

IIRC I found them to be either unreliable, unpredictable, or not quite appropriate for what I needed when writing the xtb plugin, and so instead opted to have the plugin handle merging and dropping data as appropriate.

Which of those reasons it was, I can no longer remember. But ideally future plugin authors wouldn't have to do that kind of manual adjustment.

In any case, I'd like to properly document both and define their purpose so that it's clear in future what is buggy behaviour and what is intended.

Is my understanding of their effect roughly correct? What are they actually meant to do?
What is the distinction between append and readProperties?
Are they only valid keys for menuCommand?
Would they not be better off being indicated in the TOML metadata? That's where the plugin now indicates what output Avogadro should expect, so wouldn't it make sense to also indicate there what Avogadro should do with said output?
Do we want to take the opportunity to adjust the defined intended behaviour at all, or does it make sense in both cases already?

Discuss this on our forum.

source

Comments

Sort:hotnew top

matterhorn103@discuss.avogadro.cc ⁨2⁩ ⁨months⁩ ago
Thanks for the explanations! To be clear, this is not something I’m champing at the bit to change or anything, it’s just an aspect that I’d had had on my to-do list but had never raised.

I had the idea in my head somehow that the two were more similar than they are and were somehow duplicating each other. That’s clearly not the case.

Image ghutchis:

No. It reads the supplied format directly. There’s no “converting to CJSON.”

Sorry, I meant “converting to Avogadro’s in-memory representation”, which I tend to think of as equivalent to CJSON, because both can usually represent the same information – even though of course they are not actually the same thing.

For further clarification (and this is all so that I can write clear documentation, not just to be annoying):

What happens if append is used and the original file contains properties, e.g. frequencies? Are they discarded?

What happens if append is used when the file being passed back contains properties? Presumably the properties don’t get appended too? They’re just ignored?

What happens if readProperties is used and the original file already has some property information? Are all the original properties discarded? Or are they kept unless the plugin passes replacement values?

What happens if a plugin tries to use append and readProperties at the same time?

Image ghutchis:

I guess I want to know what you found confusing and/or unreliable.

Unfortunately, I can’t exactly remember. Which I know is not helpful.

I have a vague notion that it might have been that the answer to (8) was “all the existing properties are discarded”? Because that would have had consequences for the xtb plugin – it means that if a user first calculated frequencies and then requested the MOs the frequencies would vanish.
source
- ghutchis@discuss.avogadro.cc ⁨2⁩ ⁨months⁩ ago
  If the molecular structure changes, properties should be invalidated, e.g. #2168
  
  No, it copies atoms, bonds, residues, formal charges but no properties.
  
  IIRC we had a discussion about this before, and properties should be retained unless there are replacement values (the implementation is in core/molecule.cpp)
  
  That’s not possible, because at the moment the code (interfacescript.cpp) gives precedence to readProperties
  
  Image matterhorn103:
  
  Because that would have had consequences for the xtb plugin
  
  I mean, it’s always better (more precise) if you have the cjson representation to make any specific changes you want. The main point for append and readProperties is to make it easier for plugin authors to do various things.
  
  It’s much easier to use append to implement various “Build” tools because you just add the atoms and bonds to the scene.
  source
ghutchis@discuss.avogadro.cc ⁨2⁩ ⁨months⁩ ago
Image matterhorn103:

IIRC I found them to be either unreliable, unpredictable, or not quite appropriate for what I needed when writing the xtb plugin, and so instead opted to have the plugin handle merging and dropping data as appropriate.

If you think something is unreliable, that sounds like a bug report. Granted, “append” is probably not what you want for xtb when it comes to optimizations. OTOH, for “build this nanotube and add it to the current system” it makes more sense.

Image matterhorn103:

If the file/geometry that the plugin sends back is not a CJSON, Avogadro first converts the returned format into CJSON.

No. It reads the supplied format directly. There’s no “converting to CJSON.” (unless you’re supplying something that requires Open Babel conversion which would just slow things down)

append indicates that the contents of the file (i.e., atoms and/or bonds) should be added to the current system.
readProperties was something you requested. It indicates to read properties, spectra, orbitals, etc. and add them to the current system - ignoring atoms and bonds.

See above

I’m not sure how these would apply to any other plugin type.

I don’t know why they would apply to the metadata. I can think of many, many scenarios in which you’d want to notify Avogadro only after running. I’m not really sure how it benefits Avogadro or the plugin if you want to declare append or readProperties ahead of time. Avogadro still needs to read the output JSON.

It makes sense to me, but I’m the one who wrote the code (and a bunch of plugins that use append)

I guess I want to know what you found confusing and/or unreliable. They work for my use cases certainly.
source