Skip to content
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.

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

Add release GitHub actions #336

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Add release GitHub actions #336

wants to merge 14 commits into from

Conversation

klwetstone
Copy link

@klwetstone klwetstone commented Dec 1, 2023

closes #317
closes #389

Add GitHub actions to publish on release

Do not merge - #315 is needed first. After transfer, the repo should automatically have access to the PyPI secrets needed by release.yml

ccds.__version__

Users can now see the version of ccds after importing (ccds.__version__). However, the project name is different than the module name. This means that in version.py, we have to hard code the project name with:

__version__ = importlib_metadata.version("cookiecutter-data-science")

Instead of using:

__version__ = importlib_metadata.version(__name__.split(".", 1)[0])

Using release version as default tag for checkout

Per #389, we wanted a way to allow more stable behavior when someone installs ccds. Currently, any change to the template on the master branch will automatically get propagated when someone runs ccds (unless the person has downloaded the template already and opts to skip that step). As part of this PR, we now default to ccds checking out the tag associated with the installed ccds version.

Users can always get the latest by checking out the master branch (-c master) or updating ccds.

Docs

This PR does not include versioning for docs (eg. with a "Build docs" job). That is tracked separately in #334

Copy link
Member

@jayqi jayqi left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from v2 to master May 22, 2024 16:53
ccds/__main__.py Outdated Show resolved Hide resolved
@chrisjkuch chrisjkuch marked this pull request as ready for review January 3, 2025 15:50
@chrisjkuch chrisjkuch requested a review from pjbull January 3, 2025 15:50
@jayqi jayqi self-requested a review January 3, 2025 17:38
@chrisjkuch chrisjkuch linked an issue Jan 28, 2025 that may be closed by this pull request
Copy link
Member

@pjbull pjbull left a comment

Choose a reason for hiding this comment

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

Great! Looking good with one question.

May be nice to also add some instructions on creating a release for maintainers so they know to tag the release in the right way.

ccds/__main__.py Outdated
# Per #389, set this to the currently released version by default
param_names = [p.name for p in f.params]
checkout_index = param_names.index("checkout")
f.params[checkout_index].default = __version__
Copy link
Member

Choose a reason for hiding this comment

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

It looks like some places (like the release tags), we use vX.Y.Z and others we use X.Y.Z. We should check if we need the v prefix here or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pjbull great catch, would have failed as-is without the "v" prefix. I added a RELEASING.md with some instructions, but feel free to tweak or make suggestions.

@chrisjkuch chrisjkuch assigned chrisjkuch and unassigned pjbull Jan 28, 2025
@chrisjkuch chrisjkuch requested review from pjbull and removed request for chrisjkuch February 7, 2025 12:19
@chrisjkuch chrisjkuch requested review from jayqi and r-b-g-b and removed request for jayqi February 7, 2025 12:20
Copy link
Contributor

@r-b-g-b r-b-g-b left a comment

Choose a reason for hiding this comment

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

Really nice! A couple of wording tweaks using my status as "kind of still figuring out what's going" on to write what made it click for me.

Also maybe unrelated but I notice that

ccds --version

## Cookiecutter 2.6.0 from /home/robert/projects/codepath-data-infrastructure/.venv/lib/python3.13/site-packages (Python 3.13.1 (main, Dec 19 2024, 14:32:25) [Clang 18.1.8 ])

Outputs the cookiecutter version (2.6.0), not the ccds version (2.0.1). May be inert for the time being and if so I can file an issue for another day.

HISTORY.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
RELEASING.md Outdated Show resolved Hide resolved
chrisjkuch and others added 3 commits February 7, 2025 10:45
Documentation changes

Co-authored-by: Robert Gibboni <[email protected]>
@chrisjkuch chrisjkuch requested a review from r-b-g-b February 14, 2025 13:14
@chrisjkuch
Copy link
Contributor

@r-b-g-b could you take one more look here? Applied your suggestions and added a warning about v2.0.0 per @pjbull, who has otherwise signed off

Copy link
Contributor

@r-b-g-b r-b-g-b left a comment

Choose a reason for hiding this comment

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

One minor tweak, then good to merge -- great work!

Comment on lines +6 to +12
if version <= "2.0.0":
warnings.warn(
"It looks like you're using a version of CCDS that "
"always uses the latest template. For more stable behavior, "
"update to a newer version of CCDS.",
DeprecationWarning,
)
Copy link
Contributor

@r-b-g-b r-b-g-b Feb 14, 2025

Choose a reason for hiding this comment

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

[nit] my brain found it easier to reason about < 2.0.1 than <= 2.0.0 ("if the version is less than 2.0.1, show the deprecation warning")

Then tweaked the wording a bit -- still correct?

Suggested change
if version <= "2.0.0":
warnings.warn(
"It looks like you're using a version of CCDS that "
"always uses the latest template. For more stable behavior, "
"update to a newer version of CCDS.",
DeprecationWarning,
)
if version < "2.0.1":
warnings.warn(
"It looks like you're using a version of CCDS that "
"defaults to using the latest template. For more stable behavior, "
"update to CCDS version 2.0.1 or later. To upgrade to the latest "
"version, run 'pip install -U cookiecutter-data-science'",
DeprecationWarning,
)

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to tell people how to upgrade (e.g., pip install -U cookiecutter-data-science)

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call -- updated suggestion.

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.

Release tagging mechanics Add CI for releasing package to PyPI
5 participants