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

All tests don't skip python 3.9 #3602

Merged
merged 8 commits into from
Jul 11, 2022
Merged

All tests don't skip python 3.9 #3602

merged 8 commits into from
Jul 11, 2022

Conversation

MichaelFu512
Copy link
Contributor

@MichaelFu512 MichaelFu512 commented Jul 5, 2022

Pull Request Description

Removed pytest.mark.skip_if_39 from all tests as testing them in python 3.9, none of them failed.

Did conda create -n evalml_py_39 python=3.9 then did make installdeps-dev.

After that, did a search using ctrl-f and looked for pytest.mark.skip_if_39 from all files, removed them from the files that had them and ran each tests. None of the tests that previous were skipped failed.

Only issue was two of the tests complained about not having prophet and cmdstanpy installed, but those were easily rectified by doing pip install prophet and pip install cmdstanpy.

Closes #3600


After creating the pull request: in order to pass the release_notes_updated check you will need to update the "Future Release" section of docs/source/release_notes.rst to include this pull request by adding :pr:123.

@codecov
Copy link

codecov bot commented Jul 5, 2022

Codecov Report

Merging #3602 (a9641c0) into main (931cd04) will increase coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main   #3602     +/-   ##
=======================================
+ Coverage   99.7%   99.7%   +0.1%     
=======================================
  Files        335     335             
  Lines      33522   33503     -19     
=======================================
- Hits       33393   33382     -11     
+ Misses       129     121      -8     
Impacted Files Coverage Δ
...alml/tests/component_tests/test_arima_regressor.py 100.0% <ø> (ø)
...nent_tests/test_exponential_smoothing_regressor.py 100.0% <ø> (ø)
...tests/component_tests/test_polynomial_detrender.py 100.0% <ø> (ø)
...ml/tests/component_tests/test_prophet_regressor.py 100.0% <ø> (ø)
evalml/tests/conftest.py 97.9% <ø> (+0.5%) ⬆️
.../tests/pipeline_tests/test_time_series_pipeline.py 99.9% <ø> (-<0.1%) ⬇️
evalml/tests/component_tests/test_utils.py 100.0% <100.0%> (+4.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 931cd04...a9641c0. Read the comment docs.

@MichaelFu512 MichaelFu512 marked this pull request as ready for review July 5, 2022 23:24
@MichaelFu512 MichaelFu512 enabled auto-merge (squash) July 5, 2022 23:24
Copy link
Contributor

@eccabay eccabay left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this, it's encouraging that no tests needed to be updated! I'm wondering if we should add prophet to the dev requirements - although at the moment I don't think it's necessary.

It looks like there are a few other files that have references to skipping with python 3.9:

  • evalml/tests/conftest.py
  • evalml/tests/component_tests/test_utils.py

Once those are updated as well, this should be good to go!

Copy link
Contributor

@chukarsten chukarsten left a comment

Choose a reason for hiding this comment

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

Thanks Michael! Nice job!

@MichaelFu512 MichaelFu512 changed the title All tests now run in python 3.9 All tests don't skip python 3.9 Jul 8, 2022
@MichaelFu512 MichaelFu512 enabled auto-merge (squash) July 8, 2022 20:59
Copy link
Collaborator

@jeremyliweishih jeremyliweishih left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelFu512 MichaelFu512 merged commit 57a12e7 into main Jul 11, 2022
@MichaelFu512 MichaelFu512 deleted the 3600-Undo-3.9-Testskips branch July 11, 2022 18:42
@MichaelFu512 MichaelFu512 restored the 3600-Undo-3.9-Testskips branch July 11, 2022 19:27
@chukarsten chukarsten mentioned this pull request Jul 24, 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.

Undo Skipping of Tests for Python 3.9
4 participants