Parsing fixes for ORCA xTB (Fixes #43) - #44
Conversation
|
@coltonbh If you get a chance, could you review and release this? For testing purposes, it is useful to run Orca with xTB as the method. However, the log file output shows some slight variations in this case, so this fixes those parsing issues. In particular, we dropped the |
|
Yes! Happy to release today. Will making the parser not required per my suggestion work? Then we keep the data for cases when we have it but don't raise exceptions when missing? @avcopan |
|
@coltonbh I didn't see your suggestion. Did you post it on this thread? That would probably work -- how do we do that? |
|
(Apologies if it is somewhere obvious and I missed it.) |
|
The pending review above! |
|
I don't think pending reviews show to other users. My understanding is that only you can see them. |
|
From the GitHub documentation here: 
|
|
@coltonbh Once you submit the review, I should be able to see it. |
coltonbh
left a comment
coltonbh
left a comment
There was a problem hiding this comment.
So sorry! Didn't submit!
| return match.group(1) | ||
|
|
||
|
|
||
| @register(filetype=OrcaFileType.STDOUT, target="calcinfo_natoms") |
There was a problem hiding this comment.
Maybe just make the parser optional instead?
@register(filetype=OrcaFileType.STDOUT, target="calcinfo_natoms", required=False)
This will preserve it when the the data exists but not raise an exception if not present :)
There was a problem hiding this comment.
Ideally we add a test for the xTB output logs and ensure no exception is raised. This then covers the case currently causing the error.
Thanks for this!
|
@coltonbh Sounds good! Will do! |
3 participants
@coltonbh @TroyNSmith This fixes the broken tests from PR #43.
The reason for dropping the
natomsparser is that the number of atoms line is not always printed, for example when runningxTBthrough Orca.