fix from_json methods, add from_json_file and from_dict methods

[diogomart]

Some from_json methods in take Python dictionaries as argument. These methods should be renamed to from_dict. Then, from_json methods wrap the from_dict ones. And from_json_file wrap the from_json ones.

[rwxayheee]

In molsetup.py, some are decoders (like the MoleculeSetup json decoder) named as from_json. It can be named molecule_setup_decoder, to be consistent with what's in polymer.py.

@diogomart To make the method names consistent, would you be interested in the idea of making these basic methods (to_json, to_json_file, from_json, from_json_file) inheritable from a master (base) class? Earlier, I wanted to do the same thing in another issue
#279 (comment)

I was also wondering if you might be interested in moving encoders and decoders into their own respective class? The expected keys, and the encode and decode function will be defined within the scope of the respective class. When there's a layered structure, then higher-level class can reference the encoders/decoders in the lower-level class. I'm not sure if this is common or if there's any efficiency drawback, but it might make maintenance easier. Let me know what you think, and I'm willing to take the work!

[diogomart]

Inheriting for these JSON interface methods carries the risk of having exceptional situations later and being unable to adhere to the base class. Maybe we'll have some new object for which reading from JSON doesn't make sense, then we'll choose to not inherit from it partially defeating the purpose of a base class, or have to find hacky ways to inherit anyway but not implement all the functions. At first sight I rather not inherit, but I'll think about it more... I like the features of the base class in #279

We also can't return the object passed as input (a dictionary) if the expected keys don't match: we need to raise an error. For example, if we dump a Polymer to JSON with v0.6.1 and load it again with a more recent version after PR #276 we get dictionaries instead of MoleculeSetup objects because the ring_corners attribute is not expected anymore. Raising an error would have allowed use to catch this in the tests. We need to create an exception like this one from #254

Meeko/meeko/molsetup.py

Lines 332 to 342 in 8780dd0

	# Check that all the keys we expect are in the object dictionary as a safety measure
	expected_json_keys = {"canon_id", "index1", "index2", "rotatable"}
	# the cycle break attribute was added after v0.6.1, so we are
	# defaulting to the default to allow reading .json written
	# with v0.6.0 and v0.6.0, at the expense of possibly having
	# a macrocycle broken bond that will be incorrectly set with
	# cycle_break=False, but JSON is used mostly for the receptor
	# and we don't really use macrocycle breaking for the receptor
	optional_json_keys = {"cycle_break"}
	if set(obj.keys()) - optional_json_keys != expected_json_keys:
	return obj

so that Polymer JSONs written with v0.6.1 will be supported in later versions.

[rwxayheee]

@diogomart thanks so much for your response. All this makes sense to me, and I understand it's difficult to implement at the moment.

We also can't return the object passed as input (a dictionary) if the expected keys don't match: we need to raise an error.

For example, if we dump a Polymer to JSON with v0.6.1 and load it again with a more recent version after PR #276 we get dictionaries instead of MoleculeSetup objects because the ring_corners attribute is not expected anymore.

This is currently handled by the individual decoder, but I wish the expected_keys are defined within each class like the structure in #279. Then, the inheritable methods always take these criteria from within the class.

In #279, from_json (it's the decoder, I also named it the incorrect way) has the exception handling that tries to distinguish the three major types of errors:

• partial parse, so decoder doesn't return the object but just a dict
• errors occurred within the decoder function
• parsing error (pure JSON issue)

This kind of error handling can also be made inheritable.

I appreciate your insights. We can talk more in the future, if we want to re-organize this, and let me know if I can be of some use

[diogomart]

MoleculePreparation could also read json files directly. Currently it implements from_config, which does the same as from_dict in the first post in this issue (I think we should use from_dict but keep from_config with a deprecation warning). Classmethods from_json and specially from_json_file would be nice to have. Then, this code block:

with open("my_configuration.json") as f:
    json_string = f.read()

config_dict = json.loads(json_string)
mk_prep = MoleculePreparation.from_config(config_dict)

becomes a nice one-liner:

mk_prep = MoleculePreparation.from_json_file("my_configuration.json")

[rwxayheee]

For anyone following/reading this thread, here's an update:

After the merge of #292, from_json and to_json are the major class methods for the conversion of JSON-bound objects from/to JSON strings.

See a description of the structure and relation here:
#292 (comment)

Usage and options from_json and to_json, and specially, from_dict, for lenient parsing are explained here:
#292 (comment)

Closing as resolved, but please feel free to re-open if you have any comments or suggestions regarding this.