Multiple DOIs per atomtype #168
Conversation
foyer/forcefield.py
Outdated
| @@ -721,14 +723,16 @@ def _write_references_to_file(self, atom_types, references_file): | |||
| warnings.warn("Reference not found for atom type '{}'." | |||
| "".format(atype)) | |||
| unique_references = collections.defaultdict(list) | |||
| for key, value in atomtype_references.items(): | |||
| unique_references[value].append(key) | |||
| for key, values in atomtype_references.items(): | |||
There was a problem hiding this comment.
Could we update these to be for atomtype, dois in ...?
foyer/tests/test_forcefield.py
Outdated
| with open(get_fn('ethane-multi.bib')) as file1: | ||
| with open('ethane-multi.bib') as file2: | ||
| diff = difflib.ndiff(file1.readlines(), file2.readlines()) | ||
| changes = [l for l in diff if l.startswith('+ ') or l.startswith('- ')] |
There was a problem hiding this comment.
There was a problem hiding this comment.
So I had initially tried using filecmp when writing this test for mBuild; however, I ran into an issue with Appveyor (https://ci.appveyor.com/project/ctk3b/mbuild-o0viu/build/1.0.755#L1137). I think because filecmp performs a binary comparison there is an issue that arises when comparing files generated on MacOS/Linux and on Windows (see https://bugs.python.org/issue6306).
There was a problem hiding this comment.
Ah I see - makes sense. If there is no diff, couldn't you simply assert not diff? Or will it always contain something? Is it just a header that you can unambiguously strip?
Regardless of the above, I would de-dent the changes = ... and assert lines.
There was a problem hiding this comment.
diff will show all lines, with a + in front of lines that are unique to file2 and a - in front of lines that are unique to file1. So changes is just stripping out only those lines that are different between the two files. I actually got this from this StackOverflow answer.
There was a problem hiding this comment.
Heh alright this isn't really that essential but one last suggestion: https://docs.python.org/2/library/difflib.html#difflib.unified_diff and just set n=0 to get no context lines! 🎉
There was a problem hiding this comment.
🍾 Ah, nice suggestion! It seemed like there had to be a better way to compare two files.
Currently the
doiattribute for atomtypes defined in a force field XML file only supports a single DOI. However, there may be instances where multiple DOIs would be appropriate (see #158). This PR adds support for listing multiple DOIs within thedoiattribute, and passes this information through to the BibTeX file that is created.