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

Migrate from setup.py to pyproject.toml (PEP 517 and PEP 621) #1278

Merged
merged 49 commits into from
Jan 24, 2023

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Jan 20, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • setup.py is no longer used as the packaging system, pyproject.toml with flit is now the packaging engine.
  • several configurations for linting tools (including Manifest.in) have either been rendered obsolete or been migrated into the pyproject.toml
  • TOML checkers are now present in pre-commit

What other changes need to happen?

  • package requirements need to be migrated into the package
  • user-end installation tests need to be performed
  • README and HISTORY require a pre-build step in order to properly be packaged and uploaded to PyPI. The ways this was done to date involved a hacky string replace step in setup.py. This might now need to be handled with a build-helper script
    • Drop the changelog from the PyPI index readme

Does this PR introduce a breaking change?

Yes. Very much. Installation of the package should be the same on the user end, but development tooling has changed significantly. Documentation will need to be updated accordingly.

Other information:

https://chadsmith.dev/python-packaging/

@Zeitsperre Zeitsperre added enhancement New feature or request standards / conventions Suggestions on ways forward labels Jan 20, 2023
@Zeitsperre Zeitsperre added this to the v0.41 milestone Jan 20, 2023
@Zeitsperre Zeitsperre self-assigned this Jan 20, 2023
@github-actions github-actions bot added CI Automation and Contiunous Integration docs Improvements to documenation labels Jan 20, 2023
@github-actions github-actions bot added the indicators Climate indices and indicators label Jan 20, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@Zeitsperre
Copy link
Collaborator Author

This PR is ready for final review!

Copy link
Collaborator

@aulemahal aulemahal 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!
I have an unrelated suggestion and an alternative fix.

docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
xclim/ensembles/_reduce.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the approved Approved for additional tests label Jan 24, 2023
@Zeitsperre
Copy link
Collaborator Author

@aulemahal test-py39-opt-slow is consistently giving me the following:

FAILED xclim/testing/tests/test_sdba/test_properties.py::TestProperties::test_spatial_correlogram - AssertionError: 
Not equal to tolerance rtol=1e-07, atol=0

Mismatched elements: 5 / 5 (100%)
Max absolute difference: 7.40815654e-05
Max relative difference: 4.549936e-07
 x: array([ 26.543211,  67.716254, 108.889298, 150.062341, 191.235384])
 y: array([ 26.543199,  67.716227, 108.889254, 150.062282, 191.23531 ])

Is this just a matter of adjusting tolerances, or should I be investigating further?

@github-actions github-actions bot added the sdba Issues concerning the sdba submodule. label Jan 24, 2023
@aulemahal
Copy link
Collaborator

argh.
Yes easiest would be to adjust the tolerances.

@Zeitsperre Zeitsperre merged commit 2aec12a into master Jan 24, 2023
@Zeitsperre Zeitsperre deleted the peps_517_621 branch January 24, 2023 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests CI Automation and Contiunous Integration docs Improvements to documenation enhancement New feature or request indicators Climate indices and indicators sdba Issues concerning the sdba submodule. standards / conventions Suggestions on ways forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants