New target property for specifying Python version #2332

jackyk02 · 2024-06-26T05:53:53Z

This PR introduces a new target property python-version that allows users to explicitly specify the Python version to be used.

Related Issues and PR:
Issue #2298
Issue #2299
PR #2292

Example:

target Python {
  python-version: "3.9.10"
}

If python-version is not specified, the system will default to using Python 3.10 or newer.

Summary by CodeRabbit

New Features
- Added support for specifying a specific Python version when generating Python code, defaulting to Python 3.10 or newer if no version is specified.
Improvements
- Updated the Python code generation process to allow more precise control over the Python interpreter version used, enhancing compatibility and stability.

coderabbitai · 2024-06-26T05:54:01Z

Walkthrough

The recent changes enhance the Python version compatibility handling in the PythonGenerator.java by integrating a new PythonVersionProperty to dynamically specify the required Python version. This allows the generator to default to Python 3.8 if no specific version is provided, thus improving flexibility and alignment with user requirements. Additionally, required modifications were made to the Target.java to incorporate this new property effectively.

Changes

File Path	Summary
`core/src/main/java/org/lflang/generator/python/PythonGenerator.java`	Added import for `PythonVersionProperty`, new `pythonVersion` field, modified `setUpMainTarget` to leverage `pythonVersion`.
`core/src/main/java/org/lflang/target/Target.java`	Added `PythonVersionProperty` to properties in `initialize` method for Python target.
`core/src/main/java/org/lflang/target/property/PythonVersionProperty.java`	Introduced new file defining `PythonVersionProperty` for setting a specific Python version.

Sequence Diagrams

sequenceDiagram
    participant User
    participant PythonGenerator
    participant Property

    User->>PythonGenerator: Initiate Target Setup
    PythonGenerator->>Property: Fetch PythonVersionProperty
    PythonGenerator-->>PythonGenerator: Set `pyVersion` (Default: 3.8)
    PythonGenerator->>PythonFinder: find_package(Python <pyVersion> REQUIRED COMPONENTS Interpreter Development)
    Note over User,PythonGenerator: Setup Completed with appropriate Python version

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 73e8c8a and 46d1674.

Files selected for processing (3)

core/src/main/java/org/lflang/generator/python/PythonGenerator.java (4 hunks)
core/src/main/java/org/lflang/target/Target.java (2 hunks)
core/src/main/java/org/lflang/target/property/PythonVersionProperty.java (1 hunks)

Files skipped from review due to trivial changes (1)

core/src/main/java/org/lflang/target/property/PythonVersionProperty.java

Additional comments not posted (4)

core/src/main/java/org/lflang/target/Target.java (2)

63-63: Import Statement for PythonVersionProperty Looks Good

The addition of the PythonVersionProperty import is necessary for the changes made in the initialize method for the Python target configuration.

644-645: Proper Initialization of PythonVersionProperty in Target Configuration

The addition of PythonVersionProperty to the Python target's initialization list correctly implements the functionality described in the PR. This allows users to specify the Python version, enhancing configurability.

core/src/main/java/org/lflang/generator/python/PythonGenerator.java (2)

64-64: Import Statement for PythonVersionProperty is Correct

The import of PythonVersionProperty is essential for utilizing this property in the Python code generation logic.

92-92: Addition of Static Field pythonVersion is Appropriate

The introduction of the pythonVersion static field is necessary to store the user-specified Python version. This field is used effectively in the setUpMainTarget method to determine the Python version for package configuration.

core/src/main/java/org/lflang/generator/python/PythonGenerator.java

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 46d1674 and 1d45280.

Files selected for processing (1)

core/src/main/java/org/lflang/generator/python/PythonGenerator.java (4 hunks)

Files skipped from review as they are similar to previous changes (1)

core/src/main/java/org/lflang/generator/python/PythonGenerator.java

coderabbitai

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1d45280 and 220a3ac.

Files selected for processing (3)

core/src/main/java/org/lflang/generator/python/PythonGenerator.java (5 hunks)
core/src/main/java/org/lflang/target/Target.java (2 hunks)
core/src/main/java/org/lflang/target/property/PythonVersionProperty.java (1 hunks)

Files skipped from review as they are similar to previous changes (3)

core/src/main/java/org/lflang/generator/python/PythonGenerator.java
core/src/main/java/org/lflang/target/Target.java
core/src/main/java/org/lflang/target/property/PythonVersionProperty.java

core/src/main/java/org/lflang/generator/python/PythonGenerator.java

core/src/main/java/org/lflang/target/property/PythonVersionProperty.java

core/src/main/java/org/lflang/generator/python/PythonGenerator.java

lhstrh

Thanks for addressing the feedback! Looks much better now. Please still get rid of the static variable.

lhstrh · 2024-08-03T17:17:21Z

Note also this comment.

lhstrh

Looks good now, but I'd like to discuss this feature in the developer meeting because I'd like to better understand how this would interact with virtual environments.

cmnrd · 2024-08-05T07:48:34Z

Looks like FindPython defines a variable Python_FIND_VIRTUALENV, which when set to FIRST favors the currently active virtual environment.

lhstrh · 2024-08-05T17:31:14Z

core/src/main/java/org/lflang/target/property/PythonVersionProperty.java

+    if (!version.isEmpty() && !version.contains("3.10")) {
+      reporter
+          .at(config.lookup(this), Literals.KEY_VALUE_PAIR__NAME)
+          .warning(


@cmnrd: note that we issue a warning if anything other than 3.10 is selected.

jackyk02 · 2024-08-08T03:35:18Z

I've conducted a series of tests to evaluate Python version detection in various environments:

1. Python 3.10 Virtual Environment

Created with: conda create -n py310 python=3.10
Activated with: conda activate py310

Target Python configuration:

target Python {
  python-version: "3.10"
}

Result: Correct Python interpreter found regardless of whether python-version was specified.

2. Python 3.11 Virtual Environment

Created with: conda create -n py311 python=3.11
Activated with: conda activate py311

Target Python configuration:

target Python {
  python-version: "3.11"
}

Result: Correct Python interpreter found when python-version was specified.

Note: If python-version is not specified (defaults to 3.10), the following error occurs:

-- Configuring incomplete, errors occurred!
CMake Error at /usr/share/cmake-3.30/Modules/FindPackageHandleStandardArgs.cmake:233 (message):
  Could NOT find Python (missing: Python_INCLUDE_DIRS Python_LIBRARIES
  Development Development.Module Development.Embed) (found suitable version
  "3.10.14", required range is "3.10.0...<3.11.0")

3. Global Python Environment

Assumes Python 3.10 is the interpreter determined by the $PATH environment variable
Deactivated conda environment: conda deactivate

Target Python configuration:

target Python {
  python-version: "3.10"
}

Result: Correct Python interpreter found

Based on these tests, it appears that FindPython already prioritizes the currently active virtual environment. Activating a conda virtual environment essentially applies settings to the shell to use a specific Python interpreter along with its associated modules and libraries (e.g. Python_INCLUDE_DIRS and Python_LIBRARIES).

Our current approach allows users to manage their Python environments and this PR provides an option for them to set the Python version used by LF.

cmnrd · 2024-08-09T10:13:03Z

I can confirm this also for venv with cmake 3.30 on Linux. When the virtual environment is disabled, cmake uses the system's interpreter. When it is active, it uses the interpreter from the virtual environment.

I also found out that Python_FIND_VIRTUALENV is not specified by default. I don't know what the default behavior is, perhaps it changed, but I don't think so as this is usually communicated in the cmake documentation.

coderabbitai bot reviewed Jun 26, 2024

View reviewed changes

core/src/main/java/org/lflang/generator/python/PythonGenerator.java Outdated Show resolved Hide resolved

jackyk02 added python Related to the Python target bugfix labels Jun 26, 2024

coderabbitai bot reviewed Jun 26, 2024

View reviewed changes

jackyk02 added feature New feature and removed bugfix labels Jun 26, 2024

coderabbitai bot reviewed Jun 26, 2024

View reviewed changes

jackyk02 assigned jackyk02 and unassigned jackyk02 Jun 26, 2024

lhstrh requested changes Jun 26, 2024

View reviewed changes

jackyk02 requested a review from lhstrh July 4, 2024 03:58

lhstrh reviewed Jul 4, 2024

View reviewed changes

core/src/main/java/org/lflang/generator/python/PythonGenerator.java Outdated Show resolved Hide resolved

lhstrh requested changes Jul 4, 2024

View reviewed changes

lhstrh self-requested a review August 4, 2024 04:52

lhstrh force-pushed the python-version-property branch from 59618b7 to 634bda0 Compare August 4, 2024 04:53

inital commit

42d1fee

lhstrh force-pushed the python-version-property branch from 634bda0 to d14ae87 Compare August 4, 2024 04:56

jackyk02 and others added 5 commits August 3, 2024 22:02

Update Minimum Python Version

3d35056

update generator & provide warning for unsupported versions

b0cd606

remove pythonversion from handleProtoFiles

59c1720

Update PythonVersionProperty.java

7827351

Remove static variable

1023172

lhstrh force-pushed the python-version-property branch from 10d8f07 to 1023172 Compare August 4, 2024 05:02

lhstrh reviewed Aug 4, 2024

View reviewed changes

lhstrh approved these changes Aug 4, 2024

View reviewed changes

lhstrh self-requested a review August 4, 2024 05:10

lhstrh changed the title ~~Add Target Property for Specifying Python Version~~ New target property for specifying Python version Aug 4, 2024

lhstrh reviewed Aug 5, 2024

View reviewed changes

lhstrh self-requested a review August 6, 2024 05:14

cmnrd mentioned this pull request Aug 9, 2024

Wrong Python version invoked #2298

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New target property for specifying Python version #2332

New target property for specifying Python version #2332

jackyk02 commented Jun 26, 2024 •

edited

Loading

coderabbitai bot commented Jun 26, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

lhstrh left a comment

lhstrh commented Aug 3, 2024

lhstrh left a comment

cmnrd commented Aug 5, 2024 •

edited

Loading

lhstrh Aug 5, 2024

jackyk02 commented Aug 8, 2024

cmnrd commented Aug 9, 2024

New target property for specifying Python version #2332

Are you sure you want to change the base?

New target property for specifying Python version #2332

Conversation

jackyk02 commented Jun 26, 2024 • edited Loading

Summary by CodeRabbit

coderabbitai bot commented Jun 26, 2024 • edited Loading

Walkthrough

Changes

Sequence Diagrams

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

lhstrh left a comment

Choose a reason for hiding this comment

lhstrh commented Aug 3, 2024

lhstrh left a comment

Choose a reason for hiding this comment

cmnrd commented Aug 5, 2024 • edited Loading

lhstrh Aug 5, 2024

Choose a reason for hiding this comment

jackyk02 commented Aug 8, 2024

cmnrd commented Aug 9, 2024

jackyk02 commented Jun 26, 2024 •

edited

Loading

coderabbitai bot commented Jun 26, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

cmnrd commented Aug 5, 2024 •

edited

Loading