formulate like a pkg #12

mahf708 · 2025-04-23T22:44:54Z

todo:

add hatching and init
add dynamic versioning and --version argparse stuff
add skeleton for ci (testing, docs, pypi publishing, litning, etc.)

Copilot

Pull Request Overview

This PR refactors project configuration and entry point settings while introducing dynamic versioning and updating the command line interface. Key changes include updating pyproject.toml to use dynamic versioning with Hatch, modifying the entry point from "cacts.cacts:main" to "cacts:main", and updating the signature of parse_command_line in cacts/cacts.py to accept a version parameter.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File	Description
pyproject.toml	Removed static version and related keys; added dynamic versioning and updated script entry
cacts/cacts.py	Updated main() to import version and modified parse_command_line signature
cacts/init.py	Added version and a main() function redirecting to the new entry point

Files not reviewed (1)

readme: Language not supported

Comments suppressed due to low confidence (1)

pyproject.toml:22

Confirm that the updated entry point 'cacts:main' correctly reflects the new module structure and avoids any ambiguity compared to the previous 'cacts.cacts:main'.

cacts = "cacts:main"

Copilot · 2025-04-23T22:47:34Z

cacts/cacts.py

@@ -577,7 +576,7 @@ def parse_config_file(self,config_file,machine_name,builds_types):
                    self._builds.append(build)

 ###############################################################################
-def parse_command_line(args, description):
+def parse_command_line(args, description, version):


Ensure that all invocations of parse_command_line include the version parameter so that the --version argument works as expected and to avoid potential runtime errors.

bartgol · 2025-04-23T23:08:21Z

cacts/cacts.py

@@ -635,4 +634,7 @@ def parse_command_line(args, description):
    parser.add_argument("-v", "--verbose", action="store_true",
                        help="Print output of config/build/test phases as they would be printed by running them manually.")

+    parser.add_argument("--version", action="version", version=f"%(prog)s {version}",


Does argparse handle the "print and exit" part? I don't see any logic in our code otherwise...

bartgol · 2025-04-23T23:09:38Z

cacts/__init__.py

+
+__version__ = "0.1.0"
+
+def main() -> None:


What's the diff between this entry point, and the one defined in pyproject.toml?

The main entry point should come from the top level, that's the thinking...

In the future, we could have additional cli stuff:

the main (current) will be cacti ...

in the future, we can have cacti-xyz ...

I would prefer we expose the main one through the top level. But functionally, there's zero difference

bartgol

I don't know much about packaging, so whatever you do, is prob best.

premature approval lol

mahf708 · 2025-04-23T23:25:21Z

.github/workflows/pypi.yaml

+      uses: pypa/gh-action-pypi-publish@76f52bc884231f62b9a034ebfe128415bbaabdfc
+      with:
+        user: __token__
+        password: ${{ secrets.PYPI_API_TOKEN }}


@bartgol we just need this. Basically, need the token from pypi, then we save it inside this repo as "PYPI_API_TOKEN" in the repo settings secrets/ actions/ space

See here for details: https://github.com/marketplace/actions/pypi-publish#usage

Once you add it, we can publish a test prerelease and then yank it

mahf708 · 2025-04-23T23:43:34Z

.github/workflows/test.yaml

+    strategy:
+      fail-fast: false
+      matrix:
+         python-version: ["3.6", "3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]


note to self ther ubuntu runner 22.04 only support 3.8+

mahf708 · 2025-04-24T00:06:29Z

.github/workflows/lint.yaml

+      if: ${{ matrix.linter == 'pylint' }}
+      name: Lint with pylint
+      run: |
+        pylint $(find cacts/ -name "*.py" | xargs)


@bartgol pylint and flake8 can be pretty annoying, so please take a look at the output of their complaints before approving this PR LOL

bartgol · 2025-04-24T15:07:52Z

@mahf708 what do you want to do with the py testing? Since only 3.8+ is avail, should we just rm 3.6/3.7? Of course, since CIME requires 3.6 (I think), it would be nice to test with 3.6...

mahf708 · 2025-04-24T15:21:24Z

@mahf708 what do you want to do with the py testing? Since only 3.8+ is avail, should we just rm 3.6/3.7? Of course, since CIME requires 3.6 (I think), it would be nice to test with 3.6...

let's see if running on ubuntu-20.04 will make a diff... [edit no, that's gone ... we can use some other mechanism to get 3.6, but not sure it is worth it tbh]

btw, I am going to take out those linters for now, and we can add them later. They will format most of the code... which is something I don't want us to do right now

Copilot

Pull Request Overview

This PR implements dynamic versioning, updates the build configuration (switching to hatchling), and adds CI skeleton workflows.

Updated pyproject.toml for dynamic versioning and a hatchling build backend.
Improved error handling and module imports in utility and command-line parsing code.
Added GitHub Actions workflows for testing and package publishing.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pyproject.toml	Revised project metadata, dynamic versioning, and build system settings.
cacts/utils.py	Modified the default exception type in the expect function.
cacts/machine.py	Updated Machine class initialization and variable naming.
cacts/cacts.py	Refactored command-line parsing and version import to resolve circular imports.
cacts/tests/noop_test.py	Added a basic test to ensure pytest runs.
.github/workflows/*.yaml	Introduced CI workflows for testing and PyPI publishing.

Files not reviewed (1)

readme: Language not supported

Comments suppressed due to low confidence (1)

cacts/cacts.py:644

The function 'get_current_commit()' appears undefined (consider using 'get_current_sha()') and the code following the 'return' statement is unreachable.

sha = get_current_commit()

Copilot · 2025-04-24T15:45:23Z

cacts/machine.py

        else:
-            expect (name in machines_specs.keys(),
+            avail_machs = machine_specs.keys()


The variable 'machine_specs' is likely a typo; it should be 'machines_specs' to match the constructor parameter, which may lead to a runtime error.

Suggested change

avail_machs = machine_specs.keys()

avail_machs = machines_specs.keys()

this looks like a legit typo? @bartgol

Will fix in another PR. I have to fix cdash submission anyways.

mahf708 requested review from bartgol and Copilot April 23, 2025 22:44

Copilot AI reviewed Apr 23, 2025

View reviewed changes

bartgol reviewed Apr 23, 2025

View reviewed changes

bartgol previously approved these changes Apr 23, 2025

View reviewed changes

mahf708 requested a review from bartgol April 23, 2025 23:10

bartgol approved these changes Apr 23, 2025

View reviewed changes

mahf708 commented Apr 23, 2025

View reviewed changes

mahf708 commented Apr 24, 2025

View reviewed changes

mahf708 marked this pull request as ready for review April 24, 2025 15:44

mahf708 requested review from Copilot and bartgol April 24, 2025 15:44

Copilot AI reviewed Apr 24, 2025

View reviewed changes

mahf708 and others added 3 commits April 24, 2025 11:45

formulate like a pkg

56eca30

Address some pylint errors

dc46153

add caching and drop 3.6

d669b7f

mahf708 force-pushed the mahf708/pkgmania branch from 1e4203d to d669b7f Compare April 24, 2025 15:45

bartgol approved these changes Apr 24, 2025

View reviewed changes

mahf708 merged commit 86fab2c into main Apr 24, 2025
8 checks passed

mahf708 deleted the mahf708/pkgmania branch April 24, 2025 16:32

This was referenced Jul 30, 2025

Nano bug fix when running within SLURM env #26

Closed

add linters #13

Closed

	avail_machs = machine_specs.keys()
	avail_machs = machines_specs.keys()

formulate like a pkg #12

formulate like a pkg #12

Uh oh!

Conversation

mahf708 commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Apr 23, 2025

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!

bartgol 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!

bartgol commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mahf708 commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

mahf708 Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mahf708 commented Apr 23, 2025 •

edited

Loading

bartgol commented Apr 24, 2025 •

edited

Loading

mahf708 commented Apr 24, 2025 •

edited

Loading

mahf708 Apr 24, 2025 •

edited

Loading