Skip to content

skpkg: updated labpdfproc - #228

Open
Akadib wants to merge 6 commits into
diffpy:mainfrom
Akadib:steven-updated-scikit-package
Open

Akadib wants to merge 6 commits into
diffpy:mainfrom
Akadib:steven-updated-scikit-package

Conversation

@Akadib

@Akadib Akadib commented Oct 1, 2026 •

Copy link
Copy Markdown

@sbillinge Read to review. Most of the changes made from the last PR that Rundong committed. Changed the news item to fix, deleted the auto-generated file, and link the CLI script in the project scripts within pyproject.toml file to the right name.

This PR has covered all the previous comments.

@Akadib Akadib mentioned this pull request Oct 1, 2026
@Akadib Akadib changed the title Steven updated scikit package skpkg: updated labpdfproc Oct 1, 2026
@codecov

codecov Bot commented Oct 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.17%. Comparing base (b1438f2) to head (eaa8337).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #228   +/-   ##
=======================================
  Coverage   99.17%   99.17%           
=======================================
  Files           5        5           
  Lines         364      364           
=======================================
  Hits          361      361           
  Misses          3        3           
Files with missing lines Coverage Δ
tests/test_version.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbillinge

Copy link
Copy Markdown
Contributor

@Akadib @stevenhua0320 please can you report the results of your acceptance testing? I can only merge this when I know that the program still works as expected.

Also, please discuss if you will do the documentation and requirements work on this PR or on a separate one, in which case, please create an issue that we can close.

@stevenhua0320

Copy link
Copy Markdown

@Akadib You could do this from both pytest and following the documentation of labpdfproc to run the program to test it. For pytest, please paste the result here.

@sbillinge

sbillinge commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Pytest is ok, it is handled in CI so it is not needed to be posted here. But paste here some kind of summary of the results of the integration testing (running the examples)

@stevenhua0320

stevenhua0320 commented Oct 3, 2026 •

Copy link
Copy Markdown

OK, at least when I compute these PR change on my machine it should be fine:

============================= test session starts ==============================
platform darwin -- Python 3.14.2, pytest-9.1.1, pluggy-1.6.0
rootdir: /Users/huarundong/Documents/diffpy/updated-scikit-package-7d56dfe
configfile: pyproject.toml
plugins: mock-3.16.0
collected 98 items

tests/test_fixtures.py .                                                 [  1%]
tests/test_functions.py ...............                                  [ 16%]
tests/test_tools.py .................................................... [ 69%]
.............................                                            [ 98%]
tests/test_version.py .                                                  [100%]

=============================== warnings summary ===============================
tests/test_functions.py::test_compute_cve[input_diffraction_data2-input_cve_params2]
  /Users/huarundong/Documents/diffpy/work/updated-scikit-package-7d56dfe/src/diffpy/labpdfproc/functions.py:209: UserWarning: Input mu*D = 20 is out of the acceptable range (0.5 to 7.0) for polynomial interpolation. Proceeding with brute-force computation.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================== 98 passed, 1 warning in 214.62s (0:03:34) ===================

But for learning purpose, @Akadib you should try to replicate it.

@sbillinge

Copy link
Copy Markdown
Contributor

Thanks @stevenhua0320. be clear, i am less interested in unit tests as these are passing in CI. I am asking Adib to run the program and do the examples (assuming there are examples) to learn what the program does and make sure it runs in the same way after the migration.

@stevenhua0320

Copy link
Copy Markdown

Thanks @stevenhua0320. be clear, i am less interested in unit tests as these are passing in CI. I am asking Adib to run the program and do the examples (assuming there are examples) to learn what the program does and make sure it runs in the same way after the migration.

Yeah, I had a meeting before with Adib and I think he would do it (both pytest and documentation case reproduction).

This branch has not been deployed

No deployments
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.

3 participants