CI: test that runs pdf pack examples in CI #40

cadenmyers13 · 2025-09-09T02:07:42Z

closes #8

Updated comment: This tests the example scripts by running each test in pytest infrastructure.

cadenmyers13 · 2025-09-09T02:08:44Z

.gitignore

+docs/examples/ch*/solutions/diffpy-cmi/fig/*
+docs/examples/ch*/solutions/diffpy-cmi/fit/*
+docs/examples/ch*/solutions/diffpy-cmi/res/*
+


The examples create these dirs. This prevents any accidental commits of the data

this is not good. This is precisely what we want to avoid (test-generated junk). How do the examples decide where to write these files? Can you copy the files to tmpdir and then run them?

cadenmyers13 · 2025-09-09T02:09:11Z

docs/examples/ch03NiModelling/solutions/diffpy-cmi/fitBulkNi.py

 # If we want to run using multiprocessors, we can switch this to 'True'.
 # This requires that the 'psutil' python package installed.
-RUN_PARALLEL = True
+RUN_PARALLEL = False


Requires psutil to be installed or it throws an error

We don't want to do this either. Is there a reason not to install psutil? or make it conditional on whether psutil is installed? We don't want to change the behavior of the examples just so the CI will run.

docs/examples/ch11ClusterXYZ/solutions/diffpy-cmi/fitCdSeNP.py

codecov · 2025-09-09T02:12:01Z

Codecov Report

❌ Patch coverage is 98.01980% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.95%. Comparing base (6167dd9) to head (f6ad035).

Files with missing lines	Patch %	Lines
tests/test_examples_structure.py	92.59%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main      #40       +/-   ##
===========================================
+ Coverage   50.00%   80.95%   +30.95%     
===========================================
  Files           2       17       +15     
  Lines          18     1313     +1295     
===========================================
+ Hits            9     1063     +1054     
- Misses          9      250      +241

Files with missing lines	Coverage Δ
.../ch03NiModelling/solutions/diffpy-cmi/fitBulkNi.py	`87.09% <100.00%> (ø)`
...es/ch03NiModelling/solutions/diffpy-cmi/fitNPPt.py	`85.21% <100.00%> (ø)`
...s/ch11ClusterXYZ/solutions/diffpy-cmi/fitCdSeNP.py	`89.43% <ø> (ø)`
tests/conftest.py	`70.00% <100.00%> (+34.28%)`	⬆️
tests/test_ch03.py	`100.00% <100.00%> (ø)`
tests/test_ch05.py	`100.00% <100.00%> (ø)`
tests/test_ch06.py	`100.00% <100.00%> (ø)`
tests/test_ch07.py	`100.00% <100.00%> (ø)`
tests/test_ch08.py	`100.00% <100.00%> (ø)`
tests/test_ch11.py	`100.00% <100.00%> (ø)`
... and 1 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sbillinge · 2025-09-09T17:22:13Z

Thanks @cadenmyers13 I didn't see this before, sorry about that. Thanks for looping it to me again.

cadenmyers13 · 2025-09-12T02:41:06Z

@sbillinge I tested this on my fork and the new test I wrote seem to pass (it just runs all tutorial scripts). I had to add special handling since one of the files depends on the Ni fit instrument params to get Qdamp and Qbroad. We could also just hard-code these values in but this seems to work. The downside to this test is that all the scripts take about 20 min to run on CI. This could get annoying for future PRs especially small ones. Let me know what you think of this. Im sending this message before it finishes running so hopefully it passes

cadenmyers13 · 2025-09-12T13:02:24Z

There is this codecov error being generated too. Might need to add a token

    .query commit=6d2764ef2aab012bd7fdce54ae7cb9d974b91b2b&branch=test-tutorial-CI&pr=40&build_url=http%3A%2F%2Fgithub.com%2Fdiffpy%2Fdiffpy.cmi%2Factions%2Fruns%2F17662838822&service=github-actions&build=17662838822&slug=diffpy%2Fdiffpy.cmi&yaml=.codecov.yml&package=py2.1.13
    Gzipping contents..
    Compressed contents to 1758 bytes
    Pinging Codecov...
    Retrying 1/5 in 2s..
    Retrying 2/5 in 2s..
    Retrying 3/5 in 2s..
    Retrying 4/5 in 2s..
    Retrying 5/5 in 2s..
    Direct to s3 failed. Using backup v2 endpoint.
    Uploading to Codecov...
    Retrying 1/5 in 2s..
    Retrying 2/5 in 2s..
    Retrying 3/5 in 2s..
    Retrying 4/5 in 2s..
    Retrying 5/5 in 2s..
    {'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 649s.', code='throttled')}
Error: 429 Client Error: Too Many Requests for url: https://codecov.io/upload/v2?commit=6d2764ef2aab012bd7fdce54ae7cb9d974b91b2b&branch=test-tutorial-CI&pr=40&build_url=http%3A%2F%2Fgithub.com%2Fdiffpy%2Fdiffpy.cmi%2Factions%2Fruns%2F17662838822&service=github-actions&build=17662838822&slug=diffpy%2Fdiffpy.cmi&yaml=.codecov.yml&package=py2.1.13
Tip: See all example repositories: https://github.com/codecov?query=example

sbillinge · 2025-09-13T20:46:39Z

Thanks for this @cadenmyers13 . I took a quick look and it looked as if we need to tweak it a bit but I haven't had time. Ideally we want to move everything inside pytest so it is using the teting framework and not running outside it. The testing framework is very good at setting up a kind of virtual testing environment, running all the tests, then tearing it down and deleting everything. It has to do it in a way that if anything crashes during the test, the tear-down still takes place, or your computer gets junked up with unwanted stuff. Maybe you can look again with this new view and see if it makes sense what I am saying. Another aspect of this is getting things to run on all platforms which is hard if you are using command-line calls. It is better if you can do the "command line" call by by calling the main function in the entry-point app. little tricks like that. I am not sure how well it translates to your nice script but just a few things to look for....

cadenmyers13 · 2025-09-15T20:02:08Z

@sbillinge ready for review when you've got a chance.

sbillinge

this is definitely getting better. Nice that you are using type hints.....the first in the group to do so! very advanced.....

I wonder if we want to just glob the examples directory instead of doing it this way?. This would be less work to maintain if we can figure it out because if we add new examples in the future they will automatically get tested.

sbillinge · 2025-09-17T02:40:10Z

tests/test_ch03.py

+@pytest.mark.parametrize(
+    "relative_path",
+    [
+        f"{__examples_dir__}/ch03NiModelling"


please use our pattern if you can that has a comment that mentions what is being tested and what the expected behavior is.

cadenmyers13 · 2025-09-17T15:27:01Z

Nice that you are using type hints.....the first in the group to do so! very advanced.....

I saw tieqiong doing this and thought i'd try!

I wonder if we want to just glob the examples directory instead of doing it this way?

I globbed each chapter dir. Since its now been broken out into separate tests for each chapter, there will be some maintenance if we add a chapter. But if new scripts are added to a chapter that already exists, this should run it.

@sbillinge ready for review

cadenmyers13 · 2025-09-17T22:27:33Z

@sbillinge ready for review

I added the meta-test and tested many different configurations of filenames and dir names. got expected behavior.

cadenmyers13 · 2025-09-18T15:31:50Z

docs/examples/ch03NiModelling/solutions/diffpy-cmi/fitNPPt.py

    print("Please run the Ni example first\n")
+    print("Setting Q_damp and Q_broad to refined values\n")
+    QDAMP_I = 0.045298
+    QBROAD_I = 0.016809


pytest was failing on CI (but not locally) without this. The reason why is because fitBulkNi.py refines these values, then fitNPPt.py grabs these refined values and uses them. CI cannot find these values from the Ni fit so I just pasted them here. This somewhat changes how the tutorial will be ran because the user doesn't need to fit the Ni first, but I don't feel great about adding special handling in the test for this file (potentially more maintenance in the future). Let me know if you feel differently about this.

Can we put this in a conditional, so it runs the old way but does it the new way if it can't find the other results?

yes, that is what it is doing here.

cadenmyers13 · 2025-09-18T15:34:10Z

@sbillinge I made one small change (see inline comment here https://github.com/diffpy/diffpy.cmi/pull/40/files#r2359918512). If you approve this this can be merged. Then we can release!

sbillinge

pls see comments

sbillinge · 2025-09-19T15:55:29Z

.gitignore

+docs/examples/ch*/solutions/diffpy-cmi/fig/*
+docs/examples/ch*/solutions/diffpy-cmi/fit/*
+docs/examples/ch*/solutions/diffpy-cmi/res/*
+


this is not good. This is precisely what we want to avoid (test-generated junk). How do the examples decide where to write these files? Can you copy the files to tmpdir and then run them?

sbillinge · 2025-09-19T15:57:37Z

docs/examples/ch03NiModelling/solutions/diffpy-cmi/fitBulkNi.py

 # If we want to run using multiprocessors, we can switch this to 'True'.
 # This requires that the 'psutil' python package installed.
-RUN_PARALLEL = True
+RUN_PARALLEL = False


We don't want to do this either. Is there a reason not to install psutil? or make it conditional on whether psutil is installed? We don't want to change the behavior of the examples just so the CI will run.

sbillinge · 2025-09-19T15:59:19Z

docs/examples/ch03NiModelling/solutions/diffpy-cmi/fitNPPt.py

    print("Please run the Ni example first\n")
+    print("Setting Q_damp and Q_broad to refined values\n")
+    QDAMP_I = 0.045298
+    QBROAD_I = 0.016809


Can we put this in a conditional, so it runs the old way but does it the new way if it can't find the other results?

sbillinge · 2025-09-19T17:20:19Z

tests/conftest.py

+import matplotlib
 import pytest

+__examples_dir__ = Path(__file__).parent.parent / "docs" / "examples"


we may want to cd to tmpdir copy over the examples something like that.

sbillinge · 2025-09-19T17:21:46Z

tests/test_ch03.py

+import pytest
+from conftest import __examples_dir__, run_cmi_script
+
+chapter = "ch03NiModelling"


This still is not using glob as I had in mind. Is there a reason we can't glob from the parent directory?

sbillinge · 2025-09-21T13:18:45Z

I am working on this. Weirdly, I can't get diffpy.cmi to install correctly in develop mode. It can't find the diffpy.srfit subpackage (and possibly other subpackages) even though they are correctly listed in conda list. Did you encounter anything like this?

cadenmyers13 · 2025-09-21T20:44:35Z

Weirdly, I can't get diffpy.cmi to install correctly in develop mode. It can't find the diffpy.srfit subpackage (and possibly other subpackages) even though they are correctly listed in conda list. Did you encounter anything like this?

@sbillinge Actually yes, I get the same thing. I thought it mightve been an issue with my local since I don't get this error when I install from conda-forge, but I guess not since you're getting the same thing... pip install . works fine

sbillinge · 2025-09-21T22:58:32Z

interesting. As a workaround I will do pip install . but could you see if you can get to the bottom of it?

cadenmyers13 · 2025-09-22T15:50:29Z

closing because new PR #45

cadenmyers13 · 2025-09-22T15:51:05Z

can get to the bottom of it?

@sbillinge yeah Ill check it out.

sbillinge · 2025-09-22T16:31:09Z

Nice that you are using type hints.....the first in the group to do so! very advanced.....\n\nI saw tieqiong doing this and thought i'd try!

😂, it wasn't chatgpt?

cadenmyers13 · 2025-09-22T17:52:10Z

😂, it wasn't chatgpt?

@sbillinge lol, maybe a bit of both.... 😳

cadenmyers13 · 2025-09-22T17:59:06Z

I will never openly admit it lol 😂

sbillinge · 2025-09-22T18:18:20Z

using type hints is not our group style atm, and we want standardization. However, it is a good thing in general, so we should revisit this shortly. Taking that decision means adding type hints to everything we edit on to remove technical debt, so we want to make sure the benefits outweigh the costs. It has been a struggle to bring our code to py 3.13 as you know, but now everything is pretty much updated, it may be hte right time.

cadenmyers13 · 2025-09-22T18:54:50Z

@sbillinge Yeah, using chatgpt for this could be very useful for public repos if we decide to add. still think it would take a lot of work though

sbillinge · 2025-09-23T00:39:58Z

I wouldn't use chatgpt for that. We don't have to change everything, just things we work on.

cadenmyers13 added 4 commits September 8, 2025 21:48

fix minor bugs so examples run cleanly

1e2632f

add example-generated files to gitignore

856596e

CI: add ci for testing the pdf pack

c379a11

news

522c561

cadenmyers13 commented Sep 9, 2025

View reviewed changes

docs/examples/ch11ClusterXYZ/solutions/diffpy-cmi/fitCdSeNP.py Show resolved Hide resolved

cadenmyers13 added 2 commits September 8, 2025 22:14

add requirements to CI

ac32634

fix CI bug

3840f07

cadenmyers13 added 3 commits September 9, 2025 13:37

link error

f02c948

remove separate workflow file

bce5b86

test: add new test that runs tutorial scripts

6d2764e

cadenmyers13 added 3 commits September 15, 2025 15:54

remove script that runs all examples

0ef05db

add individual test files for each example dir

4d70e15

add functions for running scripts to conftest

f1c2ac2

sbillinge reviewed Sep 17, 2025

View reviewed changes

cadenmyers13 added 2 commits September 17, 2025 11:21

glob files in examples dir

5b27183

fix typo in comment

d0e17cf

add meta-test

89e89bb

set qdamp and qbroad in example if instrument file not found

f6ad035

cadenmyers13 commented Sep 18, 2025

View reviewed changes

sbillinge reviewed Sep 19, 2025

View reviewed changes

cadenmyers13 added 2 commits September 19, 2025 17:19

test scripts in a temp dir

9408042

revert gitignore

b2fd64a

tell covecov to ignore tests, pycache, and temp dir

666707d

cadenmyers13 closed this Sep 22, 2025

cadenmyers13 deleted the test-tutorial-CI branch October 28, 2025 13:52

CI: test that runs pdf pack examples in CI #40

CI: test that runs pdf pack examples in CI #40

Uh oh!

Conversation

cadenmyers13 commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sbillinge commented Sep 9, 2025

Uh oh!

cadenmyers13 commented Sep 12, 2025

Uh oh!

cadenmyers13 commented Sep 12, 2025

Uh oh!

sbillinge commented Sep 13, 2025

Uh oh!

cadenmyers13 commented Sep 15, 2025

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cadenmyers13 commented Sep 17, 2025

Uh oh!

cadenmyers13 commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cadenmyers13 commented Sep 18, 2025

Uh oh!

sbillinge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbillinge commented Sep 21, 2025

Uh oh!

cadenmyers13 commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbillinge commented Sep 21, 2025

Uh oh!

cadenmyers13 commented Sep 22, 2025

Uh oh!

cadenmyers13 commented Sep 22, 2025

Uh oh!

sbillinge commented Sep 22, 2025

Uh oh!

cadenmyers13 commented Sep 22, 2025

Uh oh!

cadenmyers13 commented Sep 22, 2025

Uh oh!

sbillinge commented Sep 22, 2025

cadenmyers13 commented Sep 9, 2025 •

edited

Loading

codecov bot commented Sep 9, 2025 •

edited

Loading

cadenmyers13 commented Sep 21, 2025 •

edited

Loading