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

Add Python 3.10 #605

Merged
merged 9 commits into from
Apr 26, 2022
Merged

Add Python 3.10 #605

merged 9 commits into from
Apr 26, 2022

Conversation

ascillitoe
Copy link
Contributor

@ascillitoe ascillitoe commented Mar 9, 2022

Addresses #601

@ascillitoe
Copy link
Contributor Author

This is currently blocked by pytorch/pytorch#66424.

@jklaise
Copy link
Contributor

jklaise commented Mar 10, 2022

I guess technically torch is optional so we could say we support Python 3.10, but practically it wouldn't work and it would be a mess to get our CI working without the torch components. So probably best to wait.

@ascillitoe
Copy link
Contributor Author

According to the PyTorch changelog (https://github.com/pytorch/pytorch/releases) the latest pytorch version (1.11) now supports Python 3.10 (although pytorch/pytorch#66424 suggests they won't have CI for it until 1.12). Given that torch is optional we're probably good to merge this in once the alibi-testing related CI error is resolved.

@ascillitoe
Copy link
Contributor Author

ascillitoe commented Apr 14, 2022

Looks like the latest build failure is down to the typed-ast package, which doesn't play nicely with Python 3.10 on Windows. Without doing a full deps analysis, I suspect typed-ast might be being pulled in by mypy. See python/typed_ast#156 (comment) and python/mypy#10124. This might be a good time to move to mypy>=0.900, which drops typed-ast entirely (see here). Although we'll have to deal with the 3rd party library stubs change...

We might also just be able to enforce a newer typed-ast version (python/typed_ast#155 suggests the issue was recently fixed), although it still looks flaky to me...

p.s. Could also just pin the Windows/MacOS CI to 3.9 for now...

@ascillitoe ascillitoe mentioned this pull request Apr 25, 2022
@ascillitoe ascillitoe added this to the 0.7.0 milestone Apr 25, 2022
@codecov
Copy link

codecov bot commented Apr 25, 2022

Codecov Report

Merging #605 (c44fc08) into master (6ce8a51) will decrease coverage by 1.70%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #605      +/-   ##
==========================================
- Coverage   82.10%   80.39%   -1.71%     
==========================================
  Files          77       77              
  Lines       10570    10597      +27     
==========================================
- Hits         8678     8519     -159     
- Misses       1892     2078     +186     
Impacted Files Coverage Δ
alibi/explainers/tests/test_shap_wrappers.py 90.33% <100.00%> (-1.80%) ⬇️
alibi/utils/tests/test_distributed.py 56.87% <0.00%> (-37.92%) ⬇️
alibi/utils/distributed.py 30.66% <0.00%> (-36.89%) ⬇️
alibi/explainers/anchor_tabular.py 77.77% <0.00%> (-0.95%) ⬇️
alibi/explainers/anchor_base.py 86.70% <0.00%> (-0.64%) ⬇️
alibi/explainers/shap_wrappers.py 89.22% <0.00%> (-0.58%) ⬇️
alibi/api/defaults.py 100.00% <0.00%> (ø)
alibi/explainers/cfrl_base.py 88.17% <0.00%> (+0.08%) ⬆️
alibi/explainers/anchor_text.py 87.14% <0.00%> (+0.09%) ⬆️
... and 2 more

@ascillitoe
Copy link
Contributor Author

The typo explainer._check_interactions.asert_not_called() in test_shap_wrappers.py caused the following error in Python 3.10:

AttributeError: 'asert_not_called' is not a valid assertion. Use a spec for the mock if 'asert_not_called' is meant to be an attribute.

asert has now been fixed to assert.

README.md Outdated
@@ -5,7 +5,7 @@
<!--- BADGES: START --->

<!---
![Python version](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9-blue.svg)
![Python version](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg)
Copy link
Contributor

@jklaise jklaise Apr 26, 2022

Choose a reason for hiding this comment

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

Will need a rebase which should throw up another merge conflict as this has been automated on master. Not sure why it hasn't been thrown up already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because I screwed up. The above two lines are fenced inside comments, so the above change did nothing.

I actually updated the correct line (line 14) to use the automated versioning yesterday (5efc04b), so there is no need to update this manually at all now...

Will delete the confusing commented lines in master and rebase.

@ascillitoe ascillitoe merged commit 1fd0e79 into SeldonIO:master Apr 26, 2022
@jklaise jklaise mentioned this pull request May 3, 2022
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