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

CI against Python 3.12 #585

Merged
merged 11 commits into from
Oct 21, 2023
Merged

CI against Python 3.12 #585

merged 11 commits into from
Oct 21, 2023

Conversation

jGaboardi
Copy link
Member

@jGaboardi jGaboardi commented Oct 21, 2023

This PR:

  • starts testing against Python 3.12
  • drop Python 3.8 & 3.9 support
  • cleans up dips / req in pyproject.toml, ci/*, and environment.yml

Current situation:

@jGaboardi jGaboardi self-assigned this Oct 21, 2023
@jGaboardi
Copy link
Member Author

jGaboardi commented Oct 21, 2023

@jGaboardi
Copy link
Member Author

@martinfleis @sjsrey

Should we:

  1. drop 3.8 + 3.9 and have 3.10 as minimal; or
  2. only drop 3.8 and keep 3.9 as minimal

@sjsrey
Copy link
Member

sjsrey commented Oct 21, 2023

@martinfleis @sjsrey

Should we:

1. drop 3.8 + 3.9 and have 3.10 as minimal; or

2. only drop 3.8 and keep 3.9 as minimal

If we follow spec 0, 3.10 would be minimal with 3.12 out now.

Copy link
Member

@sjsrey sjsrey left a comment

Choose a reason for hiding this comment

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

lgtm

@jGaboardi jGaboardi requested a review from knaaptime October 21, 2023 17:47
@martinfleis
Copy link
Member

Can we just comment-out numba from 3.12 envs for now and merge? We can revise once numba is available.

@jGaboardi
Copy link
Member Author

Can we just comment-out numba from 3.12 envs for now and merge? We can revise once numba is available.

This sounds reasonable. I will do that and push up.

Also, I am working on #588 right now. Shall I incorporate that into this PR or in another?

@martinfleis
Copy link
Member

Let's do it as a follow-up. That one will likely see some code changes.

@jGaboardi
Copy link
Member Author

jGaboardi commented Oct 21, 2023

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #585 (a45e153) into main (0ea928a) will increase coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #585   +/-   ##
=====================================
  Coverage   84.0%   84.0%           
=====================================
  Files        128     128           
  Lines      15054   15053    -1     
=====================================
  Hits       12642   12642           
+ Misses      2412    2411    -1     

see 1 file with indirect coverage changes

@jGaboardi
Copy link
Member Author

@martinfleis this is now ready for review.

@martinfleis
Copy link
Member

martinfleis commented Oct 21, 2023

Intersting libpysal version --> Successfully installed libpysal-0.1.dev1+g344cccc

We need to ensure the tags are present. This may cause trouble. We need

- uses: actions/checkout@v4
    with:
        fetch-depth: 0 # Fetch all history for all branches and tags.

@jGaboardi
Copy link
Member Author

Add to this PR, correct?

@martinfleis
Copy link
Member

Yes please :)

@jGaboardi
Copy link
Member Author

I guess now we need to check across the ecosystem for that...

@jGaboardi
Copy link
Member Author

jGaboardi commented Oct 21, 2023

Seeing the proper libpysal version now.

  • before: libpysal-0.1.dev1+g344cccc
  • now: libpysal-4.8.1.dev19+gbb16a5eb

All passing with the exception of the known failure. The last commit to properly indent the build_docs action didn't trigger CI for some reason, but now the syntax is correct there.

@jGaboardi
Copy link
Member Author

Once this is reviewed & merged I will tackle #588.

Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

I got a branch tackling most of the warnings in CI, so that should also be resolved shortly.

@jGaboardi jGaboardi merged commit 5d09bc6 into pysal:main Oct 21, 2023
@jGaboardi jGaboardi deleted the try_312_env branch October 21, 2023 19:53
@martinfleis
Copy link
Member

@jGaboardi Why did we include xarray as a hard dependency here? Is that because we actually hard depend on it? If so we should refactor. Something around that is causing conda-forge to fail.

@jGaboardi
Copy link
Member Author

I suppose that was my oversight/mistake since there is a from_xarray() method in weights/contiguity.py and xarray is imported weights/raster.py. However, these are clearly not hard deps.

I will put in a PR to fix this immediatly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants