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

Support Python3.13 #2054

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Support Python3.13 #2054

wants to merge 14 commits into from

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Jan 20, 2025

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)
  • CHANGELOG.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?

  • Adds a Python3.13 build to the CI
  • Removes the sphinx-codeautolink/pygments workarounds

Does this PR introduce a breaking change?

No, but once this is passing, we should prepare to drop support for Python3.10. This is mentioned in CHANGELOG.rst.

Other information:

numba v0.61.0 was released earlier today: https://numba.readthedocs.io/en/stable/release/0.61.0-notes.html

I needed to add a Coveralls-based action step because the Python package for Coveralls still doesn't support Python3.13 (see: TheKevJames/coveralls-python#523)

@Zeitsperre Zeitsperre requested a review from a team January 20, 2025 17:24
@Zeitsperre Zeitsperre self-assigned this Jan 20, 2025
Copy link

Note

It appears that this Pull Request modifies the main.yml workflow.

On inspection, the XCLIM_TESTDATA_BRANCH environment variable is set to the most recent tag (v2025.1.8).

No further action is required.

@github-actions github-actions bot added the CI Automation and Contiunous Integration label Jan 20, 2025
Signed-off-by: Trevor James Smith <[email protected]>
@Zeitsperre Zeitsperre added the approved Approved for additional tests label Jan 20, 2025
Copy link
Collaborator Author

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

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

Will this trigger the advanced tests? Let's see.

It does. nice.

Signed-off-by: Trevor James Smith <[email protected]>
Signed-off-by: Trevor James Smith <[email protected]>
Signed-off-by: Trevor James Smith <[email protected]>
@Zeitsperre
Copy link
Collaborator Author

Zeitsperre commented Jan 20, 2025

It looks as though there are changes to the ways that docstrings are parsed in Python3.13: https://docs.python.org/3.13/whatsnew/3.13.html#other-language-changes

This is breaking a lot of docstring tests. Will see what I can do.

Update: This looks like it's going to be complicated. Here's the PR that added the changes: python/cpython#81283

@Zeitsperre Zeitsperre marked this pull request as draft January 20, 2025 19:43
@aulemahal
Copy link
Collaborator

@Zeitsperre This commit pushed a very simple fix to the indentation issue. I have looked at docstring_parser and it might be a good alternative. The hypothesis is that it would be stronger. My doubt are because its a new dependency. And you know, these tend to break ;) . Also, I haven't taken the time to test it.

@Zeitsperre
Copy link
Collaborator Author

I have looked at docstring_parser and it might be a good alternative. The hypothesis is that it would be stronger. My doubt are because its a new dependency. And you know, these tend to break ;) . Also, I haven't taken the time to test it.

I'm not keen to add a new requirement (though maybe that library will find use elsewhere), even if it's a transitory one (until we no longer support Python3.12 and lower, so years, I suppose). If we can extract the parsing logic that we currently have to python version-dependent functions, we'll probably be much better off.

I'll see what I can do with your approach. Thanks!

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

The new code dedents the docstrings before parsing them, which assumes that all lines of the docstrings have a common indent. This is broken when multi-line docstrings start on the first line : this was the case in sdba.properties, which I fixed in the last commit.

In linting speak, this means we want to enforce rule D213 and not D212:

D212 	Multi-line docstring summary should start at the first line
D213 	Multi-line docstring summary should start at the second line

Signed-off-by: Trevor James Smith <[email protected]>
@github-actions github-actions bot added the indicators Climate indices and indicators label Jan 24, 2025
@Zeitsperre
Copy link
Collaborator Author

@aulemahal I think I've managed to clear up a few docstring issues (and some formatting errors that have been present for a long time, too!). The remaining warnings I'm seeing arise from the docstring for robustness_fractions:

image

We might need to revisit the logic of:

robustness_fractions.__doc__ = robustness_fractions.__doc__.format(
    tests_list="{" + ", ".join(list(SIGNIFICANCE_TESTS.keys()) + ["threshold"]) + "}",
    tests_doc="\n".join(map(_gen_test_entry, SIGNIFICANCE_TESTS.items()))[
        4:  # strip first 4 chars
    ],
)

Signed-off-by: Trevor James Smith <[email protected]>
@coveralls
Copy link

Coverage Status

coverage: 89.967% (+0.03%) from 89.934%
when pulling 96d93d3 on support-python313
into d491487 on main.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the docs Improvements to documenation label Jan 24, 2025
@Zeitsperre
Copy link
Collaborator Author

Zeitsperre commented Jan 24, 2025

I'm reluctant to merge this until our documentation builds on Python3.13. The last issue is the parsing of docstrings needed for the robustness_fractions.

If I have time on Monday, I imagine the fix is quite simple (dedenting for all entries before constructing the section).

@Zeitsperre Zeitsperre marked this pull request as ready for review January 24, 2025 22:34
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 indicators Climate indices and indicators sdba Issues concerning the sdba submodule.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for numpy 2.0 and Python 3.13
3 participants