fix: restore PDFParser - #3
Conversation
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>
|
@sbillinge ready for review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ 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
🚀 New features to boost your workflow:
|
|
Thanks @cadenmyers13 . Please can you walk me through this again? I remember we did a bunch of refactoring to use |
|
@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 |
|
Right, thanks, I remember now. And Been pointed this out right? I think we have an issue on it? |
|
@sbillinge We quickly fixed it and pushed a bug release at IUCr already in srfit, so no issue exists here |
2 participants
We removed PDFParser but we actually want to keep it to keep the design of cmi the same. THis PR adds it back