From a PR review, which was not a blocker for the initial implementation
Let me be a little more clear - what it would mean to merge here is not explicitly covered in the spec. I think this implementation is reasonable, though, so I'm just nudging for tests to cover that.
Deeper:
- If the sections differ in the model file they're based off of, I can't imagine a meaningful way to merge them. Erroring is right and what happens
- I figure it's un-/under-defined to try to have multiple copies of this section, and hope the API prevents that from happening (from disk, in-memory, registering a new handler, etc.). This should be true for other handlers as well? This hypothetical doesn't make any sense for the other parameters
- If they're identical, then it doesn't really matter which is used.
- Even the existing spec ("provided the top-level tags have compatible attributes") leaves a little room for art
Originally posted by @mattwthompson in openforcefield/openff-toolkit#2048 (comment)
From a PR review, which was not a blocker for the initial implementation
Let me be a little more clear - what it would mean to merge here is not explicitly covered in the spec. I think this implementation is reasonable, though, so I'm just nudging for tests to cover that.
Deeper:
Originally posted by @mattwthompson in openforcefield/openff-toolkit#2048 (comment)