Skip to content

Fix: Restore extensibility of ProfileParser and use load_data from diffpy.utils - #196

Open
cadenmyers13 wants to merge 13 commits into
diffpy:v3.3.0from
cadenmyers13:fix-parser
Open

Fix: Restore extensibility of ProfileParser and use load_data from diffpy.utils#196
cadenmyers13 wants to merge 13 commits into
diffpy:v3.3.0from
cadenmyers13:fix-parser

Conversation

@cadenmyers13

Copy link
Copy Markdown
Contributor

closes #194

This PR reworks ProfileParser so metadata and data parsing are extensible per format:

  • ProfileParser.parse_file is now a template method backed by _parse_metadata/_parse_data. If a user wants to create their own parser, they have the option to parse data and metadata separately with the respective private functions.
  • PDFParser is now identical to ProfileParser which uses load_data. I think this is okay for now, but we might want to reconsider in the future.
  • parse_file now accepts load_data's keyword arguments (usecols, delimiter, comments, etc.), so files with more than four columns can be read by selecting columns explicitly.
  • PDFContribution.loadData now takes a file name only (string/open-file input has been removed, consistent with the rest of the parser).
  • parseString is removed from ProfileParser and PDFParser, since parsers now read from a file.
  • The si-q27r60-xray.gr and ni-q27r100-neutron.gr test files were updated from 2008-era PDFgetX2/PDFgetN headers to modern diffpy.pdfgetx/xPDFsuite header formats (data values unchanged), and examples/docs were updated to use the new parser.

Comment thread tests/test_pdf.py
# ----------------------------------------------------------------------------


def testParser1(datafile):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reworked this old test to be up to our standards. See below

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.26471% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.89%. Comparing base (0b16bfd) to head (cd002d3).
⚠️ Report is 5 commits behind head on v3.3.0.

Files with missing lines Patch % Lines
tests/test_sas.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           v3.3.0     #196      +/-   ##
==========================================
+ Coverage   79.08%   81.89%   +2.80%     
==========================================
  Files          25       25              
  Lines        3821     3927     +106     
==========================================
+ Hits         3022     3216     +194     
+ Misses        799      711      -88     
Files with missing lines Coverage Δ
tests/test_fitrecipe.py 99.85% <100.00%> (+<0.01%) ⬆️
tests/test_pdf.py 54.28% <100.00%> (+2.11%) ⬆️
tests/test_profileparser.py 100.00% <100.00%> (ø)
tests/test_sas.py 15.78% <0.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cadenmyers13

Copy link
Copy Markdown
Contributor Author

@sbillinge ready for review. See the news file and my above message for a good summary of whats been changed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant