Skip to content

skpkg: getting package up to scikit-package CI standards #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

skpkg: getting package up to scikit-package CI standards #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6977f3e
skpkg: mirate src folder
cadenmyers13 Jun 10, 2025
bd4fa97
skpkg: migrate tests folder
cadenmyers13 Jun 10, 2025
d292578
skpkg: list dependencies in requirements folder
cadenmyers13 Jun 10, 2025
3227e2f
skpkg: list dependencies in requirements folder
cadenmyers13 Jun 10, 2025
cd84158
skpkg: add pyproject.toml
cadenmyers13 Jun 10, 2025
9170c23
skpkg: add CI and issue/PR templates
cadenmyers13 Jun 10, 2025
bd3f896
chore: news
cadenmyers13 Jun 10, 2025
09b3d8f
chore: news pt2
cadenmyers13 Jun 10, 2025
e5ad3ab
[pre-commit.ci] auto fixes from pre-commit hooks
pre-commit-ci[bot] Jun 10, 2025
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: 1 addition & 4 deletions .github/workflows/tests-on-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
name: Tests on PR

on:
push:
branches:
- main
pull_request:
workflow_dispatch:

jobs:
tests-on-pr:
uses: Billingegroup/release-scripts/.github/workflows/_tests-on-pr.yml@v0
uses: scikit-package/release-scripts/.github/workflows/_tests-on-pr.yml@v0
with:
project: diffpy.utils
c_extension: false
Expand Down
8 changes: 1 addition & 7 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ __pycache__/
.Python
env/
build/
_build/
develop-eggs/
dist/
downloads/
Expand Down Expand Up @@ -90,10 +91,3 @@ target/

# Ipython Notebook
.ipynb_checkpoints

# version information
setup.cfg
/src/diffpy/*/version.cfg

# Rever
rever/
24 changes: 24 additions & 0 deletions news/setup-CI.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* Support ``scikit-package`` Level 5 standard for file structure and CI workflow (https://scikit-package.github.io/scikit-package/).


**Security:**

* <news item>
10 changes: 5 additions & 5 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,28 @@ build-backend = "setuptools.build_meta"
name = "diffpy.utils"
dynamic=['version', 'dependencies']
authors = [
{ name="Simon J.L. Billinge group", email="simon.billinge@gmail.com" },
{ name="Simon Billinge", email="sb2896@columbia.edu" },
]
maintainers = [
{ name="Simon J.L. Billinge group", email="simon.billinge@gmail.com" },
{ name="Simon Billinge", email="sb2896@columbia.edu" },
]
description = "General utilities for analyzing diffraction data"
keywords = ["text data parsers", "wx grid", "diffraction objects"]
keywords = ['text data parsers', 'wx grid', 'diffraction objects']
readme = "README.rst"
requires-python = ">=3.11, <3.14"
classifiers = [
'Development Status :: 5 - Production/Stable',
'Environment :: Console',
'Intended Audience :: Developers',
'Intended Audience :: Science/Research',
'License :: Free To Use But Restricted',
'License :: OSI Approved :: BSD License',
Copy link
Contributor

Choose a reason for hiding this comment

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

please check the license to make sure it is BSD before accepting this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbillinge confirmed BSD from skpkg

Copy link
Contributor

Choose a reason for hiding this comment

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

The relevant license is the diffpy.utils license not the skpkg license. What is the current diffpy.utils license.

'Operating System :: MacOS :: MacOS X',
'Operating System :: Microsoft :: Windows',
'Operating System :: POSIX',
'Operating System :: Unix',
'Programming Language :: Python :: 3.11',
'Programming Language :: Python :: 3.12',
'Programming Language :: Python :: 3.13',
'Programming Language :: Python :: 3.13',
'Topic :: Scientific/Engineering :: Physics',
'Topic :: Scientific/Engineering :: Chemistry',
]
Expand Down
Empty file removed requirements/build.txt
View file
Open in desktop
Empty file.
17 changes: 1 addition & 16 deletions src/diffpy/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
#!/usr/bin/env python
##############################################################################
#
# diffpy by DANSE Diffraction group
# Simon J. L. Billinge
# (c) 2010 The Trustees of Columbia University
# in the City of New York. All rights reserved.
# (c) 2024 The Trustees of Columbia University in the City of New York.
# (c) 2025 The Trustees of Columbia University in the City of New York.
# All rights reserved.
#
# File coded by: Billinge Group members and community contributors.
Expand All @@ -16,14 +12,3 @@
# See LICENSE.rst for license information.
#
##############################################################################
"""diffpy - tools for structure analysis by diffraction.
Copy link
Contributor

Choose a reason for hiding this comment

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

its unlikely that we want to delete the contents of this file. But I am not sure. Is this a new development that we can handle the namespace strcture (diffpy.something) without this extend path? We may want to ask @bobleesj

Copy link
Contributor

Choose a reason for hiding this comment

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

An empty src/<namespacename>/__init__.py is still needed.

The namespace's``init.py, it's written in post_gen_hook.py` shown below:

def __gen_init__(module_name):
    """Generate __init__.py file for namespace module."""
    __init__ = f"""#!/usr/bin/env python
##############################################################################
#
# (c) {% now 'utc', '%Y' %} The Trustees of Columbia University in the City of New York.
# All rights reserved.
#
# File coded by: Billinge Group members and community contributors.
#
# See GitHub contributions for a more detailed list of contributors.
# https://github.com/{{ cookiecutter.github_username_or_orgname }}/{{ cookiecutter.github_repo_name }}/graphs/contributors
#
# See LICENSE.rst for license information.
#
##############################################################################

""" # noqa: E999
    return __init__


Blank namespace package.
"""


from pkgutil import extend_path

__path__ = extend_path(__path__, __name__)

# End of file
10 changes: 6 additions & 4 deletions src/diffpy/utils/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
#!/usr/bin/env python
##############################################################################
#
# (c) 2024 The Trustees of Columbia University in the City of New York.
# (c) 2025 The Trustees of Columbia University in the City of New York.
# All rights reserved.
#
# File coded by: Billinge Group members and community contributors.
# File coded by: Simon Billinge, Billinge Group members.
#
# See GitHub contributions for a more detailed list of contributors.
# https://github.com/diffpy/diffpy.utils/graphs/contributors
#
# See LICENSE.rst for license information.
#
##############################################################################
"""Shared utilities for diffpy packages."""
"""General utilities for analyzing diffraction data."""

# package version
from diffpy.utils.version import __version__
from diffpy.utils.version import __version__ # noqa

# silence the pyflakes syntax checker
assert __version__ or True

# End of file
8 changes: 5 additions & 3 deletions src/diffpy/utils/version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#!/usr/bin/env python
##############################################################################
#
# (c) 2024 The Trustees of Columbia University in the City of New York.
# (c) 2025 The Trustees of Columbia University in the City of New York.
# All rights reserved.
#
# File coded by: Billinge Group members and community contributors.
# File coded by: Simon Billinge, Billinge Group members.
#
# See GitHub contributions for a more detailed list of contributors.
# https://github.com/diffpy/diffpy.utils/graphs/contributors
# https://github.com/diffpy/diffpy.utils/graphs/contributors # noqa: E501
#
# See LICENSE.rst for license information.
#
Expand All @@ -21,3 +21,5 @@
from importlib.metadata import version

__version__ = version("diffpy.utils")

# End of file
101 changes: 4 additions & 97 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,112 +1,19 @@
import json
from pathlib import Path

import numpy as np
import pytest

from diffpy.utils.diffraction_objects import DiffractionObject


@pytest.fixture
def user_filesystem(tmp_path):
base_dir = Path(tmp_path)
home_dir = base_dir / "home_dir"
home_dir.mkdir(parents=True, exist_ok=True)
cwd_dir = home_dir / "cwd_dir"
cwd_dir = base_dir / "cwd_dir"
cwd_dir.mkdir(parents=True, exist_ok=True)
home_config_data = {
"owner_name": "home_ownername",
"owner_email": "home@email.com",
"owner_orcid": "home_orcid",
}

home_config_data = {"username": "home_username", "email": "home@email.com"}
with open(home_dir / "diffpyconfig.json", "w") as f:
json.dump(home_config_data, f)
yield home_dir, cwd_dir


@pytest.fixture
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to delete all our tests?

def datafile():
"""Fixture to dynamically load any test file."""
base_path = Path(__file__).parent / "testdata" # Adjusted base path

def _load(filename):
return base_path / filename

return _load


@pytest.fixture
def do_minimal():
# Create an instance of DiffractionObject with empty xarray and yarray
# values, and a non-empty wavelength
return DiffractionObject(
xarray=np.empty(0), yarray=np.empty(0), xtype="tth", wavelength=1.54
)


@pytest.fixture
def do_minimal_tth():
# Create an instance of DiffractionObject with non-empty xarray, yarray,
# and wavelength values
return DiffractionObject(
wavelength=2 * np.pi,
xarray=np.array([30, 60]),
yarray=np.array([1, 2]),
xtype="tth",
)


@pytest.fixture
def do_minimal_d():
# Create an instance of DiffractionObject with non-empty xarray, yarray,
# and wavelength values
return DiffractionObject(
wavelength=1.54,
xarray=np.array([1, 2]),
yarray=np.array([1, 2]),
xtype="d",
)


@pytest.fixture
def wavelength_warning_msg():
return (
"No wavelength has been specified. You can continue to use the "
"DiffractionObject, but some of its powerful features will not be "
"available. "
"To specify a wavelength, if you have "
"do = DiffractionObject(xarray, yarray, 'tth'), "
"you may set do.wavelength = 1.54 for a wavelength of 1.54 angstroms."
)


@pytest.fixture
def invalid_q_or_d_or_wavelength_error_msg():
return (
"The supplied input array and wavelength will result in an "
"impossible two-theta. "
"Please check these values and re-instantiate the DiffractionObject "
"with correct values."
)


@pytest.fixture
def invalid_add_type_error_msg():
return (
"You may only add a DiffractionObject with another DiffractionObject "
"or a scalar value. "
"Please rerun by adding another DiffractionObject instance or a "
"scalar value. "
"e.g., my_do_1 + my_do_2 or my_do + 10 or 10 + my_do"
)


@pytest.fixture
def x_values_not_equal_error_msg():
return (
"The two objects have different values in x arrays "
"(my_do.all_arrays[:, [1, 2, 3]]). "
"Please ensure the x values of the two objects are identical "
"by re-instantiating the DiffractionObject with the correct x value "
"inputs."
)
yield tmp_path
2 changes: 1 addition & 1 deletion tests/test_version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Unit tests for __version__.py."""

import diffpy.utils
import diffpy.utils # noqa
Copy link
Contributor

Choose a reason for hiding this comment

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

It is probably ok to leave these # noqa's but they are not needed. I think we need them in the template because there is jinja code there that black can;t handle but after the template is built it would be better if they were deleted (but not hte end of theworld if they stay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbillinge I'll investigate to see if there is a way to remove this. There might be a shell script we can run that removes this after the package is created. Something like grep -rn # noqa

Copy link
Contributor

Choose a reason for hiding this comment

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

no shell script, but maybe something in the post-gen-hook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bobleesj Do you have any insight on how this might be done?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add # noqa? The cookiecutter template doesn't require it:

"""Unit tests for __version__.py."""

import {{ cookiecutter.package_dir_name }}  # noqa


def test_package_version():
    """Ensure the package version is defined and not set to the initial
    placeholder."""
    assert hasattr({{ cookiecutter.package_dir_name }}, "__version__")
    assert {{ cookiecutter.package_dir_name }}.__version__ != "0.0.0"
``



def test_package_version():
Expand Down
Loading