fix: restore PDFParser - #3
sbillinge merged 1 commit into
Conversation
Sorry, something went wrong.
The package __init__ imported diffpy.cmipdf.pdfparser and listed PDFParser in __all__, but the module had been removed, so `import diffpy.cmipdf` raised ModuleNotFoundError and the whole test suite failed at collection. Port pdfparser.py from diffpy.srfit, which is the authoritative copy: its PDFParser was refactored onto ProfileParser's parse_file template with _parse_metadata/_parse_data hooks and gained free-text instrument comment parsing for NOMAD files. No deprecation shims are carried over, since diffpy.cmipdf has never been released. tests/test_parser.py exercised diffpy.srfit's ProfileParser and asserted the pre-refactor behaviour that dx is an array of zeros; it is replaced by tests/test_pdfparser.py, ported from diffpy.srfit, which tests this package's PDFParser and expects dx to be None. Metadata expectations are adapted to this repo's synthetic .gr fixtures rather than overwriting them. Adds nom-mno-neutron.gr for the NOMAD comment-header case and the as_list fixture the ported tests need. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #3 +/- ##
===========================================
+ Coverage 53.06% 87.11% +34.05%
===========================================
Files 3 5 +2
Lines 245 194 -51
===========================================
+ Hits 130 169 +39
+ Misses 115 25 -90
|
Sorry, something went wrong.
|
Thanks @cadenmyers13 . Please can you walk me through this again? I remember we did a bunch of refactoring to use load_data instead of this parser, but now we have to add this back? What is the reason in detail? That seems like it may be a retrograde step but I guess there is probably a reason, but it smells a bit like a workaround and not a fix.....? |
Sorry, something went wrong.
|
@sbillinge ProfileParser in cmi is designed such that you can extend off of it and make your own. The base profile parser uses load_data, but say your data file isn't compatible with load_data then you can design your own. We did not want to deviate from this design. Also, PDFParser is able to parse neutron data from the NOMAD beamline which Ben was using. There could be others that also use it for different files. Btw, this was copied directly from what we have in srfit currently |
Sorry, something went wrong.
|
Right, thanks, I remember now. And Been pointed this out right? I think we have an issue on it? |
Sorry, something went wrong.
|
@sbillinge We quickly fixed it and pushed a bug release at IUCr already in srfit, so no issue exists here |
Sorry, something went wrong.
We removed PDFParser but we actually want to keep it to keep the design of cmi the same. THis PR adds it back