Skip to content
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ __pycache__/
# C extensions
*.so

# ignore any data saved by the diffpy.cmi examples
docs/examples/ch*/solutions/diffpy-cmi/fig/*
docs/examples/ch*/solutions/diffpy-cmi/fit/*
docs/examples/ch*/solutions/diffpy-cmi/res/*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

# Distribution / packaging
.Python
env/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@

# 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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Requires psutil to be installed or it throws an error

Copy link
Contributor

Choose a reason for hiding this comment

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

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.



# Functions that will carry out the refinement ##################
Expand Down
3 changes: 3 additions & 0 deletions docs/examples/ch03NiModelling/solutions/diffpy-cmi/fitNPPt.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@
print("The Ni example refines instrument parameters\n")
print("The instrument parameters are necessary to run this fit\n")
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that is what it is doing here.


# If we want to run using multiprocessors, we can switch this to 'True'.
# This requires that the 'psutil' python package installed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,6 @@ def plot_results(recipe, figname):
diff = g - gcalc + diffzero

mpl.rcParams.update(mpl.rcParamsDefault)
plt.style.use(
str(PWD.parent.parent.parent / "utils" / "billinge.mplstyle")
)

fig, ax1 = plt.subplots(1, 1)

Expand Down
2 changes: 1 addition & 1 deletion docs/source/tutorials/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Despite its utility, PDF fitting and analysis can be challenging.
The process of understand PDFs requires time, care, and the development of intuition to connect structural models to experimental data.
Hopefully by the end of this tutorial series, PDF fitting will go from being a mysterious black box to a powerful tool in your structural analysis.

Example usage of ``diffpy.cmi`` can be found at `this GitHub repo <https://github.com/diffpy/pdfttp_data>`_.
Example usage of ``diffpy.cmi`` can be found at `this GitHub repo <https://github.com/Billingegroup/pdfttp_data>`_.

.. _structural-parameters:

Expand Down
23 changes: 23 additions & 0 deletions news/test-tutorial-CI.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* Add CI for testing examples of the PDF pack.

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
26 changes: 26 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import importlib.util
import json
from pathlib import Path

import matplotlib
import pytest

__examples_dir__ = Path(__file__).parent.parent / "docs" / "examples"
Copy link
Contributor

Choose a reason for hiding this comment

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

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



@pytest.fixture
def user_filesystem(tmp_path):
Expand All @@ -17,3 +21,25 @@ def user_filesystem(tmp_path):
json.dump(home_config_data, f)

yield tmp_path


@pytest.fixture(scope="session", autouse=True)
def use_headless_matplotlib():
"""Force matplotlib to use a headless backend during tests."""
matplotlib.use("Agg")


def load_module_from_path(path: Path):
"""Load a module given an absolute Path."""
spec = importlib.util.spec_from_file_location(path.stem, path)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module


def run_cmi_script(script_path: Path):
"""General runner for example scripts with a main()."""
module = load_module_from_path(script_path)
if not hasattr(module, "main"):
pytest.skip(f"{script_path} has no main() function")
module.main()
15 changes: 15 additions & 0 deletions tests/test_ch03.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path

import pytest
from conftest import __examples_dir__, run_cmi_script

chapter = "ch03NiModelling"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

chapter_dir = Path(__examples_dir__) / chapter
example_scripts = list(chapter_dir.rglob("*.py"))


# Runs all example scripts in chapter 3, skips files if main() is not defined.
# Passes if script runs without error.
@pytest.mark.parametrize("script_path", example_scripts, ids=lambda p: p.name)
def test_ch03_examples(script_path):
run_cmi_script(script_path)
15 changes: 15 additions & 0 deletions tests/test_ch05.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path

import pytest
from conftest import __examples_dir__, run_cmi_script

chapter = "ch05Fit2Phase"
chapter_dir = Path(__examples_dir__) / chapter
example_scripts = list(chapter_dir.rglob("*.py"))


# Runs all example scripts in chapter 5, skips files if main() is not defined.
# Passes if script runs without error.
@pytest.mark.parametrize("script_path", example_scripts, ids=lambda p: p.name)
def test_ch03_examples(script_path):
run_cmi_script(script_path)
15 changes: 15 additions & 0 deletions tests/test_ch06.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path

import pytest
from conftest import __examples_dir__, run_cmi_script

chapter = "ch06RefineCrystalStructureGen"
chapter_dir = Path(__examples_dir__) / chapter
example_scripts = list(chapter_dir.rglob("*.py"))


# Runs all example scripts in chapter 6, skips files if main() is not defined.
# Passes if script runs without error.
@pytest.mark.parametrize("script_path", example_scripts, ids=lambda p: p.name)
def test_ch03_examples(script_path):
run_cmi_script(script_path)
15 changes: 15 additions & 0 deletions tests/test_ch07.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path

import pytest
from conftest import __examples_dir__, run_cmi_script

chapter = "ch07StructuralPhaseTransitions"
chapter_dir = Path(__examples_dir__) / chapter
example_scripts = list(chapter_dir.rglob("*.py"))


# Runs all example scripts in chapter 7, skips files if main() is not defined.
# Passes if script runs without error.
@pytest.mark.parametrize("script_path", example_scripts, ids=lambda p: p.name)
def test_ch03_examples(script_path):
run_cmi_script(script_path)
15 changes: 15 additions & 0 deletions tests/test_ch08.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path

import pytest
from conftest import __examples_dir__, run_cmi_script

chapter = "ch08NPRefinement"
chapter_dir = Path(__examples_dir__) / chapter
example_scripts = list(chapter_dir.rglob("*.py"))


# Runs all example scripts in chapter 8, skips files if main() is not defined.
# Passes if script runs without error.
@pytest.mark.parametrize("script_path", example_scripts, ids=lambda p: p.name)
def test_ch03_examples(script_path):
run_cmi_script(script_path)
15 changes: 15 additions & 0 deletions tests/test_ch11.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from pathlib import Path

import pytest
from conftest import __examples_dir__, run_cmi_script

chapter = "ch11ClusterXYZ"
chapter_dir = Path(__examples_dir__) / chapter
example_scripts = list(chapter_dir.rglob("*.py"))


# Runs all example scripts in chapter 11, skips files if main() is not defined.
# Passes if script runs without error.
@pytest.mark.parametrize("script_path", example_scripts, ids=lambda p: p.name)
def test_ch03_examples(script_path):
run_cmi_script(script_path)
55 changes: 55 additions & 0 deletions tests/test_examples_structure.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import re
from pathlib import Path

EXAMPLES_DIR = Path(__file__).parent.parent / "docs" / "examples"
TESTS_DIR = Path(__file__).parent


def get_example_chapters():
"""Return a dict {chapter_name: Path} for all dirs in examples/."""
return {d.name: d for d in EXAMPLES_DIR.iterdir() if d.is_dir()}


def get_test_chapters():
"""Return a set of real chapter names that have test files."""
chapters = set()
for test_file in TESTS_DIR.glob("test_ch*.py"):
m = re.match(r"test_(ch\d+)\.py", test_file.name)
if m:
prefix = m.group(1)
match = next(
(
d.name
for d in EXAMPLES_DIR.iterdir()
if d.name.startswith(prefix)
),
None,
)
if match:
chapters.add(match)
return chapters


def test_chapters_have_valid_names_and_tests():
"""Check that all example chapters are named properly and have
tests."""
example_chapters = get_example_chapters()
test_chapters = get_test_chapters()
errors = []
# 1. Fail if any dir does not follow "ch" + number convention
bad_names = [
name for name in example_chapters if not re.match(r"^ch\d+", name)
]
if bad_names:
errors.append(
"Invalid example chapter names (must match '^ch[0-9]+'): "
+ str(bad_names)
)
# 2. Fail if any example chapter has no corresponding test
missing_tests = set(example_chapters) - test_chapters
if missing_tests:
errors.append(
"Missing test files for chapters or bad test file name "
"(must be named 'test_ch<NN>.py'): " + str(sorted(missing_tests))
)
assert not errors, "\n".join(errors)
Loading