Skip to content

fix a bug : when sklearn has string characters in its version, sklear…#68

Open
earlsuke wants to merge 1 commit into
nok:stablefrom
earlsuke:fix_bug_related_with_version_conversion
Open

fix a bug : when sklearn has string characters in its version, sklear…#68
earlsuke wants to merge 1 commit into
nok:stablefrom
earlsuke:fix_bug_related_with_version_conversion

Conversation

@earlsuke

Copy link
Copy Markdown

…n-porter raises a value error
#67

Comment thread sklearn_porter/Porter.py

## Suffix lists for minor, patch version in sklearn
MINOR_VERSION_SUFFIX_LIST = ["rc3", "rc2", "rc1", "b1", "a1"]
PATCH_VERSION_SUFFIX_LIST = ["post1", "0b1", "0b2"]

@SoftwareApe SoftwareApe Apr 22, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

At least post1 is not part of the patch version, but after the patch version:

grafik

Comment thread sklearn_porter/Porter.py
Comment on lines +60 to +93

## Suffix lists for minor, patch version in sklearn
MINOR_VERSION_SUFFIX_LIST = ["rc3", "rc2", "rc1", "b1", "a1"]
PATCH_VERSION_SUFFIX_LIST = ["post1", "0b1", "0b2"]

sklearn_ver = str(sklearn_ver).split('.')
sklearn_ver = [int(v) for v in sklearn_ver]
major, minor = sklearn_ver[0], sklearn_ver[1]
patch = sklearn_ver[2] if len(sklearn_ver) >= 3 else 0

major_cand_str = sklearn_ver[0]
minor_cand_str = sklearn_ver[1]
patch_cand_str = sklearn_ver[2] if len(sklearn_ver) >= 3 else '0'

## Convert version str to integer
major = int(major_cand_str)

try:
minor = int(minor_cand_str)

except ValueError as ve:
for check_key in MINOR_VERSION_SUFFIX_LIST:
if check_key in minor_cand_str:
minor_cand_str.replace(check_key, '')
break
minor = int(minor_cand_str)

try:
patch = int(patch_cand_str)

except ValueError as ve:
for check_key in PATCH_VERSION_SUFFIX_LIST:
if check_key in patch_cand_str:
patch_cand_str.replace(check_key, '0')
break
patch = int(patch_cand_str)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems like a perfect application for regex.

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.

2 participants