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

Update supported python versions #64

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

altheaden
Copy link
Collaborator

conda-forge has recently dropped support for python 3.8 and gained support for python 3.12, so I am updated these versions in this PR.

@altheaden altheaden requested a review from xylar September 9, 2024 19:38
@altheaden
Copy link
Collaborator Author

I haven't done any testing yet, but I can if desired. I am not sure if it's needed for these changes. I added a file in the ci folder for 3.12, but it's just based on the prior files (3.9 to 3.11), so it might not be set up correctly. I see that CI is failing for 3.12 so maybe there's something else I need to add to this, but I'm not sure what.

@xylar
Copy link
Collaborator

xylar commented Sep 10, 2024

@altheaden and I discussed that we will first try to update the GitHub Actions workflow to no longer use mamba/mambaforge and see if that fixes python 3.12.

@xylar
Copy link
Collaborator

xylar commented Sep 10, 2024

@altheaden, can you rebase this one now that I merged #65?

@altheaden altheaden force-pushed the update-python-versions branch from a614aa1 to eab1c11 Compare September 10, 2024 18:06
@xylar xylar self-requested a review September 10, 2024 18:17
@xylar
Copy link
Collaborator

xylar commented Sep 10, 2024

@altheaden, I think something is still wrong with the workflow here. This step:

- if: ${{ steps.skip_check.outputs.should_skip != 'true' }}
name: Install pyremap
run: |
conda install --file dev-spec.txt
python -m pip install .

should use conda create like we do on mache:
https://github.com/E3SM-Project/mache/blob/main/.github/workflows/build_workflow.yml#L96-L102
Could you see if you can make that change? I think using conda install is not behaving well.

@altheaden altheaden force-pushed the update-python-versions branch from eab1c11 to 5799998 Compare September 10, 2024 19:59
@xylar
Copy link
Collaborator

xylar commented Sep 10, 2024

@altheaden, I think we might be able to fix the current issues by removing these lines:

          # IMPORTANT: This needs to be set for caching to work properly!
          use-only-tar-bz2: true

Despite the comment, this is no longer needed.

@altheaden altheaden force-pushed the update-python-versions branch from 5799998 to 2cb49ce Compare September 10, 2024 20:35
@altheaden
Copy link
Collaborator Author

@xylar I assume you have already looked at the changes I made in the build workflow (to match mache's, minus the -e in the pip command). You were right about the "important" line (haha). I think everything should be working now.

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

Okay, great! I think we're finally in business.

@xylar
Copy link
Collaborator

xylar commented Sep 10, 2024

@xylar I assume you have already looked at the changes I made in the build workflow (to match mache's, minus the -e in the pip command). You were right about the "important" line (haha). I think everything should be working now.

Yes, as I think we discussed, the -e doesn't make sense in CI (thought it does no harm). So this looks great!

@xylar xylar merged commit 4748924 into MPAS-Dev:main Sep 10, 2024
5 checks passed
@xylar
Copy link
Collaborator

xylar commented Sep 10, 2024

Thanks for the hard work on this @altheaden!

@altheaden altheaden deleted the update-python-versions branch September 10, 2024 20:47
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.

2 participants