Update actions - #171
Conversation
Reviewer's GuideThis PR modernizes the repository’s CI and formatting infrastructure by upgrading GitHub Actions workflows, replacing legacy pre-commit hooks and lint settings with a new Ruff-based stack, and applying the resulting automatic formatting fixes across the codebase. Entity relationship diagram for updated pre-commit configurationerDiagram
PRE_COMMIT_CONFIG ||--|{ REPO : contains
REPO {
string url
string rev
string[] hooks
}
HOOK {
string id
string[] args
string[] exclude_types
}
PRE_COMMIT_CONFIG {
REPO[] repos
}
REPO ||--|{ HOOK : has
PRE_COMMIT_CONFIG ||--|{ RUFF_CONFIG : uses
RUFF_CONFIG {
int line_length
string[] ignore
string[] select
string[] known_first_party
string[] known_third_party
string[] per_file_ignores
}
Class diagram for updated ConditionalFlowMatcher and subclassesclassDiagram
class ConditionalFlowMatcher {
+float sigma
+__init__(sigma: Union[float, int] = 0.0)
+compute_lambda(t)
}
class ExactOptimalTransportConditionalFlowMatcher {
+OTPlanSampler ot_sampler
+__init__(sigma: Union[float, int] = 0.0)
+sample_location_and_conditional_flow()
}
class TargetConditionalFlowMatcher {
+compute_mu_t()
+compute_sigma_t()
+compute_conditional_flow()
}
class SchrodingerBridgeConditionalFlowMatcher {
+__init__(sigma: Union[float, int] = 1.0, ot_method="exact")
+compute_sigma_t()
+compute_conditional_flow()
+sample_location_and_conditional_flow()
}
class VariancePreservingConditionalFlowMatcher {
+compute_mu_t()
+compute_conditional_flow()
}
ConditionalFlowMatcher <|-- ExactOptimalTransportConditionalFlowMatcher
ConditionalFlowMatcher <|-- TargetConditionalFlowMatcher
ConditionalFlowMatcher <|-- SchrodingerBridgeConditionalFlowMatcher
ConditionalFlowMatcher <|-- VariancePreservingConditionalFlowMatcher
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Double-check that switching the GitHub action to Python 3.13 is supported by all your dependencies and runners, as 3.13 may not yet be stable in some environments.
- Since bandit was removed in favor of gitleaks, consider re-adding a dedicated Python security linter or enabling ruff’s security plugins to retain deeper vulnerability scanning.
- Verify that renaming the workflow to code-quality.yaml and adding pull_request triggers on main and release/* still aligns with your CI/CD expectations and fires correctly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Double-check that switching the GitHub action to Python 3.13 is supported by all your dependencies and runners, as 3.13 may not yet be stable in some environments.
- Since bandit was removed in favor of gitleaks, consider re-adding a dedicated Python security linter or enabling ruff’s security plugins to retain deeper vulnerability scanning.
- Verify that renaming the workflow to code-quality.yaml and adding pull_request triggers on main and release/* still aligns with your CI/CD expectations and fires correctly.
## Individual Comments
### Comment 1
<location> `tests/test_models.py:8` </location>
<code_context>
def test_initialize_models():
- model = UNetModel(
+ UNetModel(
dim=(1, 28, 28),
num_channels=32,
</code_context>
<issue_to_address>
Missing assertions in model initialization test.
Add assertions to verify correct model instantiation and expected key properties, such as output shapes and parameter counts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| from torchcfm.models import MLP | ||
| from torchcfm.models.unet import UNetModel | ||
|
|
||
|
|
There was a problem hiding this comment.
suggestion (testing): Missing assertions in model initialization test.
Add assertions to verify correct model instantiation and expected key properties, such as output shapes and parameter counts.
| @@ -119,7 +120,7 @@ def plot_paths(self): | |||
| plt.show() | |||
|
|
|||
| def factory(name, args): | |||
There was a problem hiding this comment.
issue (code-quality): The first argument to instance methods should be self (instance-method-first-arg-name)
Github actions for this repo have not been updated in a while. This PR seeks to update to a more modern workflow.
ruff,gitleaks,prettierSummary by Sourcery
Modernize the repository’s code quality and CI tooling by upgrading pre-commit hooks and GitHub Actions workflows, adopting a new formatting stack (ruff, gitleaks, prettier), and applying automated fixes and cleanup across the codebase.
Enhancements:
CI:
Documentation:
Tests:
Chores: