Skip to content

deprecate: Deprecate function in ProfileParser and deprecate PDFParser#171

Open
cadenmyers13 wants to merge 20 commits intodiffpy:v3.3.0from
cadenmyers13:profileparser_dep
Open

deprecate: Deprecate function in ProfileParser and deprecate PDFParser#171
cadenmyers13 wants to merge 20 commits intodiffpy:v3.3.0from
cadenmyers13:profileparser_dep

Conversation

@cadenmyers13
Copy link
Contributor

As I was working on deprecation, I felt as if we could centralize the parser to one place and use load_data from diffpy.utils instead of having separate parsers.

The PDFParser was designed for older data formats too so it doesn't capture as much info as shown here:

PDFParser metadata: for data.gr

{'bank': 0,
 'filename': PosixPath('data.gr'),
 'nbanks': 1,
 'qmax': 25.0,
 'qmin': 0.1,
 'stype': 'X'}

ProfileParser metadata: for data.gr

{'backgroundfile': 'Xiaoyu_empty_kapton_PDF_20251021-124540_f33625_sum.iq',
 'backgroundfilefull': '/Users/cadenmyers/billingelab/BNL/oct2025_beamtime/data/Xiaoyu/iq/Xiaoyu_empty_kapton_PDF_20251021-124540_f33625_sum.iq',
 'bank': 0,
 'bgscale': 1.0,
 'composition': 'TiSe2',
 'dataformat': 'QA',
 'filename': 'data.gr',
 'inputdir': '/Users/cadenmyers/billingelab/BNL/oct2025_beamtime/data/Xiaoyu/iq',
 'inputfile': 'TiSe2_RT_XS2_intercalated_PDF_20251021-124651_85419c_sum.iq',
 'mode': 'xray',
 'nbanks': 1,
 'outputtype': 'gr',
 'qmax': 25.0,
 'qmaxinst': 25.0,
 'qmin': 0.1,
 'rmax': 140.0,
 'rmin': 0.0,
 'rpoly': 0.7,
 'rstep': 0.01,
 'savedir': '/Users/cadenmyers/billingelab/BNL/oct2025_beamtime/data/Xiaoyu/gr',
 'wavelength': 0.1}

The work here deprecates PDFParser in favor of ProfileParser, a more general approach to parsing data. I've included tests for this too. This also centralizes some parsing functionality which will hopefully make it easier to dev on and easier for users. Im open to discussion if you'd prefer a different route though.

From what i inferred from the code, it seems like ProfileParser was designed so people can build their own parsers for their specific data formats and PDFParser is one example that does this.

- For files with 3 columns: assumes (x, y, dy) and sets dx to 0.
- For files with 4 columns: assumes (x, y, dx, dy).
- For other cases: `column_format` must be explicitly specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here in the docstring describes the default behavior, users can change this depending on their data format

@cadenmyers13
Copy link
Contributor Author

@sbillinge ready for review. It looks like precommit is failing because of some python 3.14 thing

@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 98.41270% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.89%. Comparing base (ec2b2af) to head (ab44228).
⚠️ Report is 1 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     #171      +/-   ##
==========================================
+ Coverage   78.60%   78.89%   +0.29%     
==========================================
  Files          24       25       +1     
  Lines        3734     3786      +52     
==========================================
+ Hits         2935     2987      +52     
  Misses        799      799              
Files with missing lines Coverage Δ
tests/conftest.py 92.89% <100.00%> (+0.49%) ⬆️
tests/test_contribution.py 99.58% <ø> (ø)
tests/test_fitrecipe.py 99.85% <100.00%> (ø)
tests/test_pdf.py 43.58% <100.00%> (+0.29%) ⬆️
tests/test_profile.py 99.52% <100.00%> (+0.03%) ⬆️
tests/test_profileparser.py 100.00% <100.00%> (ø)
tests/test_weakrefcallable.py 98.91% <ø> (ø)
tests/test_sas.py 15.78% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cadenmyers13
Copy link
Contributor Author

cadenmyers13 commented Mar 17, 2026

@sbillinge I figured out the pre-commit issue. its failing because docformatter is not compatible with python3.14 yet and pre-commit.ci is already running on python3.14. The way we can fix this is by pinning language_version: python3.13 in docformatter,

  - repo: https://github.com/PyCQA/docformatter
    rev: v1.7.7
    hooks:
      - id: docformatter
        language_version: python3.13
        additional_dependencies: [tomli]
        args: [--in-place, --config, ./pyproject.toml]

This will have to be removed in the future or we can just let pre-commit fail for now until docformatter is up to 3.14 standards.

It fails because some .s attribute was deprecated and now removed in favor of .value. Looks like this issue is already addressed here : scikit-package/scikit-package#640

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