Skip to content

skpkg: use package update to run skpkg on files#87

Open
zmx27 wants to merge 5 commits intodiffpy:mainfrom
zmx27:package-update
Open

skpkg: use package update to run skpkg on files#87
zmx27 wants to merge 5 commits intodiffpy:mainfrom
zmx27:package-update

Conversation

@zmx27
Copy link
Copy Markdown
Contributor

@zmx27 zmx27 commented Apr 3, 2026

No description provided.

@zmx27
Copy link
Copy Markdown
Contributor Author

zmx27 commented Apr 3, 2026

@sbillinge ready for review

Copy link
Copy Markdown
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

nice work! thanks @zmx27. Please see the comments. I think it is fine to merge after these are addressed. I asked @vincefn about the app. In general, I would like to keep that in there just so it is easier to do these scikit-package updates in the future and I don't think it does any harm.

BSD 3-Clause License

Copyright (c) 2025, The Trustees of Columbia University in the City of New York.
Copyright (c) 2025-2026, The Trustees of Columbia University in the City of New York.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

something is wrong here. The dates should go back to much earlier, so I think something got lost over time. Also, we are moving to assigning copyright to "contributors to " and having the dates as -present

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I checked the git history of this file and it seems like this file has not been modified since this repo first got skpkg back in 2025. To address the dates issue, I will simply copy paste the contents of the LICENSE.rst file, which contains that go back to 2009. However, could you clarify what you mean by assigning copyright to "contributors to "? So for example, would I replace The Trustees of Columbia University in the City of New York. with Contributors to pyobjcryst here??

##############################################################################
#
# (c) 2025 The Trustees of Columbia University in the City of New York.
# (c) 2025-2026 The Trustees of Columbia University in the City of New York.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please see comment above about dates.

LICENSE.rst Outdated
Copyright (c) 2014-2019, Brookhaven Science Associates, Brookhaven National Laboratory

Copyright (c) 2024-2025, The Trustees of Columbia University in the City of New York.
Copyright (c) 2024-2026, The Trustees of Columbia University in the City of New York.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dates

exclude = [] # exclude packages matching these glob patterns (empty by default)
namespaces = false # to disable scanning PEP 420 namespaces (true by default)

[project.scripts]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check that this exists and that we want it. @vincefn pyobjcryst is just a library and doesn't need an app, but by default in scikit-package that we are using now, if no app exists it seeds the project with a simple app that can be run from the command line and just returns the version of the program.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This indeed exists in src/pyobjcryst/pyobjcryst_app.py.

@zmx27
Copy link
Copy Markdown
Contributor Author

zmx27 commented Apr 3, 2026

@sbillinge ready for review

Copy link
Copy Markdown
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

please see comment, the copyright statement is not quite right and needs to be updated everywhere.


Copyright (c) 2014-2019, Brookhaven Science Associates, Brookhaven National Laboratory

Copyright (c) 2024-Present, The Trustees of Columbia University in the City of New York.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't seem right. I think it should be 2024-2025 here, then 2025-present pyobjcryst contributors. Please check that the statement is like this everywhere

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.

2 participants