Update python version #233
Conversation
Walkup is not supported in python 3.11:
File "/Users/robin/Documents/git/MEDS_transforms/MIMIC-IV_Example/pre_MEDS.py", line 266, in main
relative_in_fp = fp.relative_to(out_fp.resolve().parent, walk_up=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
WalkthroughThe pull request modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant System
User->>System: Requests to run project
System-->>User: Checks Python version
alt Python version >= 3.12
System-->>User: Runs project successfully
else Python version < 3.12
System-->>User: Error - Python version not supported
end
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
pyproject.toml (2)
13-13: Update documentation to reflect Python version requirementThe Python version requirement change should be documented to help users understand the upgrade path.
Please ensure this change is reflected in:
- README.md
- Installation instructions
- Migration guide (if applicable)
Would you like me to help create a migration guide that includes:
- Reason for the version update
- Steps to upgrade
- Known compatibility issues
- Workarounds for users who cannot upgrade immediately
13-13: Consider adding upper bound to Python version requirementThe current specification
">=3.12"allows any future Python version, which might lead to compatibility issues.Consider using a more specific version range:
-requires-python = ">=3.12" +requires-python = ">=3.12,<3.13"This ensures compatibility is maintained until the code is verified with Python 3.13.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
13-13: Verify if Python 3.12 resolves the walk_up parameter issue
While updating to Python 3.12 might resolve the issue, let's verify if this is the most appropriate solution for the walk_up parameter problem in relative_to.
Consider documenting alternative approaches for Python 3.11 users or providing a compatibility layer, as forcing an upgrade to 3.12 might be disruptive for some users.
✅ Verification successful
Python 3.12 requirement is justified for the walk_up parameter
The codebase shows a direct dependency on the walk_up parameter in MIMIC-IV_Example/pre_MEDS.py, where it's used for creating relative symlinks. This parameter was introduced in Python 3.12's Path.relative_to() method, making the Python 3.12 requirement necessary for the functionality to work correctly.
MIMIC-IV_Example/pre_MEDS.py: Usesrelative_to(out_fp.resolve().parent, walk_up=True)for symlink creation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for relative_to usage and related issues
# Search for relative_to usage in the codebase
echo "Searching for relative_to usage..."
rg "relative_to\(" -A 2 -B 2
# Check if there are any existing workarounds or alternative approaches
echo "Checking for potential alternative approaches..."
rg "walk_up|walkup" -A 2 -B 2
Length of output: 10419
| @@ -10,7 +10,7 @@ authors = [ | |||
| ] | |||
| description = "MEDS ETL and transformation functions leveraging a sharding-based parallelism model & polars." | |||
| readme = "README.md" | |||
| requires-python = ">=3.11" | |||
| requires-python = ">=3.12" | |||
There was a problem hiding this comment.
💡 Codebase verification
Python 3.12 requirement needs reconsideration
The version update to Python 3.12 raises several concerns:
- All core dependencies (polars, pyarrow, nested_ragged_tensors) are compatible with Python 3.11
- The
walk_upparameter forPath.relative_to()was actually introduced in Python 3.12, but it's only used in an example file (MIMIC-IV_Example/pre_MEDS.py) - Forcing Python 3.12 might unnecessarily restrict users when the codebase could work with 3.11
Recommendations:
- Keep Python 3.11 requirement and modify the example file to use a different approach for relative path handling
- Or document the Python 3.12 requirement specifically for the MIMIC-IV example in its README
🔗 Analysis chain
Verify dependency compatibility with Python 3.12
The update to Python 3.12 might impact compatibility with project dependencies. Let's verify the compatibility status of critical dependencies.
Let me check the actual dependencies specified in the project to ensure we have a complete picture.
Let me check if the walkup parameter issue in Path.relative_to is actually resolved in Python 3.12, as this is the stated reason for the version bump.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Python 3.12 compatibility for key dependencies
# Extract dependencies and check their metadata
for pkg in polars pyarrow nested_ragged_tensors meds; do
echo "Checking $pkg..."
curl -s "https://pypi.org/pypi/$pkg/json" | \
jq -r '.info | "Latest version: " + .version + "\nPython requires: " + .requires_python'
done
Length of output: 1165
Script:
#!/bin/bash
# Get all dependencies from pyproject.toml
cat pyproject.toml | grep -A 50 "dependencies = \["
Length of output: 2631
Script:
#!/bin/bash
# Search for usage of relative_to with walk_up parameter
rg "relative_to.*walk_up" -A 2
Length of output: 269
|
@rvandewater as we deprecate these example folders and move to the ETL template repos like this for MIMIC, do we still need to address this issue? https://github.com/mmcdermott/MIMIC_IV_MEDS |
|
In case the MIMIC_IV pipeline has not changed we would need to specify python 3.12 for that specific etl. Otherwise, no we do not. |
Doing the MIMIC-IV example in Python 3.11 gives me:
Summary by CodeRabbit
New Features
Bug Fixes