Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Feature Constructor: Compatibility with Python 3.8 #4222

Merged
merged 1 commit into from
Nov 29, 2019

Conversation

lanzagar
Copy link
Contributor

Issue

Feature Constructor did not work on Python 3.8 due to AST changes.

Description of changes
  • Add the newly required argument posonlyargs
    As far as I can tell, this does not cause any problems on older versions of python and is silently ignored (always empty anyway).
  • Add support for new expression type Constant (which is replacing Num, Str, ...)

@ajdapretnar
Copy link
Contributor

This seems to work fine on Py3.7.

@codecov
Copy link

codecov bot commented Nov 26, 2019

Codecov Report

Merging #4222 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4222      +/-   ##
==========================================
- Coverage   85.95%   85.95%   -0.01%     
==========================================
  Files         394      394              
  Lines       70164    70164              
==========================================
- Hits        60310    60309       -1     
- Misses       9854     9855       +1

@lanzagar
Copy link
Contributor Author

@ales-erjavec can you check the code in case I missed something before we merge?

Copy link
Contributor

@ales-erjavec ales-erjavec left a comment

Choose a reason for hiding this comment

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

Seems OK.

@janezd janezd merged commit 4127f20 into biolab:master Nov 29, 2019
@lanzagar lanzagar deleted the featconst-py38 branch November 29, 2019 11:16
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.

4 participants