Skip to content

Conversation

@DimitriPapadopoulos
Copy link
Contributor

No description provided.

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review July 9, 2025 19:14
@martindurant
Copy link
Member

Indeed 3.8 is EOL, and I support dropping any mention to it. However, this PR does not drop support for it, but changes the installed python version for one of the CI checking runs - a run which is not currently invoked at all.

@martindurant
Copy link
Member

(the changelog suggests 3.8 support was dropped in code in 2025.3.2)

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Jul 10, 2025

Then how about removing file ci/environment-typecheck.yml? And ci/environment-py38.yml? Or all of ci?

I came across this file while grep'ping for 3.8 through the repository to solve an issue with Python 3.8 while preparing #1887. I don't even know what the ci directory is about, but the occurrence of 3.8 in it is confusing.

I wrote "Drop support" because I see this issue as a left-over from the PR that did drop support for 3.8. I will change the title.

@DimitriPapadopoulos DimitriPapadopoulos changed the title Drop support for Python 3.8 Remove any references to Python 3.8 Jul 10, 2025
Copy link
Member

Choose a reason for hiding this comment

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

OK, fine, we rename it. This didn't actually change anything, though!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, py38 feels definitely wrong, py39 is less wrong. Perhaps rename to py, since this file is not directly related to the minimal Python version?

Copy link
Member

Choose a reason for hiding this comment

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

The value of the python version is overidden in the GHA

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's why keeping a version of Python in this file name is misleading.

Copy link
Member

Choose a reason for hiding this comment

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

Right, so let's change it to something that works!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps linux, as it's the name of the associated CI job.

jobs:
linux:
name: ${{ matrix.PY }}-pytest
runs-on: ubuntu-24.04

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the name is *-pytest , the dictionary key doesn't seem to matter :)
But yes, linux would be fine.

@martindurant
Copy link
Member

Feel free to remove ci/environment-typecheck.yml

@martindurant
Copy link
Member

All good!

@martindurant martindurant merged commit 57b7381 into fsspec:master Jul 14, 2025
10 checks passed
@DimitriPapadopoulos DimitriPapadopoulos deleted the python_3.8 branch July 14, 2025 19:13
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