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

Transition to pyproject.toml and setup.cfg #3494

Merged
merged 35 commits into from
May 25, 2022
Merged

Transition to pyproject.toml and setup.cfg #3494

merged 35 commits into from
May 25, 2022

Conversation

eccabay
Copy link
Contributor

@eccabay eccabay commented May 2, 2022

Closes #3412

Also removes the concept of core-requirements within evalml. The core-requirements file still exists should users want to download evalml without catboost and xgboost, but there are no more tests or integrations for it. Ideally we'll remove this entirely in the future.

@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #3494 (c6ea39d) into main (04f1388) will decrease coverage by 0.1%.
The diff coverage is 98.2%.

@@           Coverage Diff           @@
##            main   #3494     +/-   ##
=======================================
- Coverage   99.7%   99.7%   -0.0%     
=======================================
  Files        336     335      -1     
  Lines      33415   33263    -152     
=======================================
- Hits       33292   33135    -157     
- Misses       123     128      +5     
Impacted Files Coverage Δ
...ests/automl_tests/test_automl_search_regression.py 95.2% <ø> (-0.2%) ⬇️
...valml/tests/automl_tests/test_default_algorithm.py 100.0% <ø> (ø)
evalml/tests/conftest.py 97.5% <ø> (-<0.1%) ⬇️
...s/prediction_explanations_tests/test_algorithms.py 100.0% <ø> (ø)
...understanding_tests/test_permutation_importance.py 100.0% <ø> (ø)
...valml/tests/pipeline_tests/test_component_graph.py 99.9% <ø> (-<0.1%) ⬇️
evalml/utils/__init__.py 100.0% <ø> (ø)
evalml/tests/pipeline_tests/test_pipelines.py 99.8% <80.0%> (-<0.1%) ⬇️
evalml/utils/cli_utils.py 96.9% <90.5%> (-3.1%) ⬇️
evalml/automl/automl_search.py 99.6% <100.0%> (-<0.1%) ⬇️
... and 16 more

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 04f1388...c6ea39d. Read the comment docs.

@gsheni
Copy link
Contributor

gsheni commented May 3, 2022

@eccabay Glad to see this happen for EvalML. You can delete MANIFEST.in and move it to setup.cfg

  • alteryx/featuretools@74bfdb4
    You can also move additional things to pyproject.toml (like pytest, isort, coverage run/report). See above link

setup.cfg Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
multi_line_output=3
skip=__init__.py

[darglint]
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be moved to pyproject.toml

setup.py Outdated Show resolved Hide resolved
@eccabay eccabay marked this pull request as ready for review May 12, 2022 13:39
@freddyaboulton
Copy link
Contributor

@eccabay I think we also need to update release.yml to use version 2 of the upload to pypi action like woodwork did here:
alteryx/woodwork@e81dc64#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R13

MANIFEST.in Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
docs-requirements.txt Show resolved Hide resolved
tools/format_release_notes.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thank you @eccabay for sticking with this and thank you @gsheni for helping out for the review. Left some small comments. Should be good to merge after that.

setup.cfg Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@@ -585,10 +585,6 @@ def __init__(
self.random_seed = random_seed
self.n_jobs = n_jobs

if not self.plot and self.verbose:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plotly moved from being a core-requirement to one of the regular requirements. self.plot is only None if plotly isn't installed. Since we no longer test this case, I removed this to make codecov happy (and users that pip install us shouldn't have an issue regardless)

evalml/automl/utils.py Outdated Show resolved Hide resolved
.github/workflows/install_test.yaml Show resolved Hide resolved
evalml/utils/cli_utils.py Show resolved Hide resolved
Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Looks good to me @eccabay !

@chukarsten chukarsten merged commit 0b7d2a7 into main May 25, 2022
@chukarsten chukarsten deleted the 3412_pyproject branch May 25, 2022 19:07
@gsheni gsheni mentioned this pull request Jun 1, 2022
@freddyaboulton freddyaboulton mentioned this pull request Jun 9, 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.

Transition EvalML to use pyproject.toml and setup.cfg (away from setup.py)
4 participants