images icon indicating copy to clipboard operation
images copied to clipboard

Use non-deprecated alias for util.status_iterator

Open j9ac9k opened this issue 2 years ago • 5 comments

sphinx.util.status_iterator alias has been deprecated since sphinx 6.1 and will be removed in sphinx 8.0

See https://www.sphinx-doc.org/en/master/extdev/deprecated.html

The preferred access for status_iterator is sphinx.util.display.status_iterator. This commit adopts the new sphinx API and then falls back on the older API access since this plugin supports older versions of sphinx.

j9ac9k avatar Oct 29 '23 16:10 j9ac9k

@jonascj this looks like a good change, could you merge this PR?

SiboVG avatar May 31 '24 17:05 SiboVG

Sure, I'll have a look at it one of the coming days. The package needs a new release and have for some time https://github.com/sphinx-contrib/images/issues/33 .

jonascj avatar Jun 03 '24 10:06 jonascj

It looks like Sphinx 8.0 did indeed remove the deprecated alias: https://www.sphinx-doc.org/en/master/changes.html#release-8-0-0-released-jul-29-2024

aiudirog avatar Jul 30 '24 16:07 aiudirog

How days turns to weeks ...

I've outlined the tasks which needs completion to do a new release, this is obviously on the todo-list since it is required for newer Sphinx versions: https://github.com/sphinx-contrib/images/issues/40

jonascj avatar Aug 03 '24 20:08 jonascj

I've got the same problem with Sphinx 8, this code fix it.
Do you know when this PR can be merged ?

Hadrien45 avatar Oct 08 '24 09:10 Hadrien45

bump

t-b avatar Jan 21 '25 20:01 t-b

Bump +1

ajaybhargav avatar Feb 12 '25 09:02 ajaybhargav

When will this be able to be merged? I was hoping to use the sphinxcontrib-images that hasn't been maintained for 4 years.

TriangleBear avatar Mar 26 '25 02:03 TriangleBear

Bump +1

Bump +2 😄

boryssmejda avatar Mar 31 '25 13:03 boryssmejda

@TriangleBear @boryssmejda A PR to fix CI / testing would be very welcome. Since this PR apparently looked good to merge mid 2024 CI / testing has stopped working (I don't remember why I didn't just merge it back then - maybe CI had just started not working).

I played around with the CI workflow yesterday https://github.com/sphinx-contrib/images/tree/ci-tox-tests . It turned out to be, for me at least, a can of worms. The workflow yml-file needs updating. Action versions @v2 are deprecated and need updating to @v3 or @v4, tox config whitelist_externals have changed to allowlist_externals, setup.py should no longer be invoked to build packages and a few other, similar surprises.

I'll give it another go, this time focusing on making the old package build with the old versions of sphinx and python. It was probably a mistake to throw away the 2.7, 3.6, 3.7 and 3.8 tests yesterday and introduce python versions 3.10 - 3.13.

jonascj avatar Mar 31 '25 21:03 jonascj

Much of the CI work is actually handled in https://github.com/sphinx-contrib/images/pull/41 . But maybe it jumps the gun on newer spinx and python versions or maybe everything will be fine if #39 and #41 are just merged, eyes somewhat closed.

jonascj avatar Mar 31 '25 21:03 jonascj

@jonascj can you test both merge in another separate branch first, if that's possible. As much as a want to work with it I'm still new to this.

TriangleBear avatar Mar 31 '25 23:03 TriangleBear

@jonascj would you like me to rebase this PR, I'll be happy to do so. As @TriangleBear suggested, if you wanted to test this PR with another one, best way to do it is merge create a new branch that has the commits of both #39 and #41

git remote add terencehonles https://github.com/terencehonles/sphinx-contrib-images
git fetch terencehonles
git remote add j9ac9k https://github.com/j9ac9k/images
git fetch j9ac9k
git switch update-packaging-and-testing-and-drop-eol-pythons
git switch -c testing-39-and-41
git rebase -i use-non-deprecated-sphinx-api
git push --set-upstream origin/testing-39-and-41

j9ac9k avatar Apr 01 '25 03:04 j9ac9k

@TriangleBear @j9ac9k I've tested merging #41 followed by merging #39 in this branch https://github.com/sphinx-contrib/images/tree/pr3941 and everything looks good (https://github.com/sphinx-contrib/images/actions/runs/14230225566).

Would you test https://test.pypi.org/project/sphinxcontrib-images/0.9.5rc1/ to see if everything is working as expected?

python -m venv venv
source venv/bin/activate
pip install -i https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple/ sphinxcontrib-images==0.9.5rc1

For me the package is installable and it can build the documentation in docs/ , but produces some non-fatal errors while doing so. Maybe it is just the remote images (wikipedia's logo) which are not longer available. Maybe it is something else, I'll investigate tomorrow.

I haven't really considered the version requirements suggested in #41 in details, we should consider those a second time. Dropping support for Python<3.9 seems reasonable, but does that necessitate dropping support for sphinx 4?

Not that it matters much, but should it be version 0.9.5 or something else, like 1.0.0? I suppose the extension does have a public API which calls for 1.y.z (same applied in 2021 though), but bumbing the major number is normally reserved for breaking backward compability.

jonascj avatar Apr 02 '25 23:04 jonascj

FYI pip can install directly from a branch on github

pip install git+https://github.com/sphinx-contrib/images@pr3941

~~I did a quick run on docs for pyqtgraph, I got an error, but I think it's unrelated to this extension (more likely having to do w/ the jump to sphinx 8). I'll debug a bit more and report back.~~ EDIT: realized I didn't initialize the submodule, did find an issue with the brown import (posted in the other PR). After changing brown to 'brown' everything is working as expected.

I'm reluctant to weigh in much on what versions of libraries your project should support, but since I'm the author of this PR and you asked here, I'll add some commentary. In the pydata/numpy ecosystem it's been recognized that supporting older pythons, numpy's etc has taken a huge toll on maintainers and they first rolled out NEP-29 (NEPs are numpy's versions of PEPs) which has now been super-seeded by SPEC-0. Both these specifications give a timeline for when support should be dropped for older versions of packages in the ecosystem (as well as python itself). Per SPEC-0, for new major/minor releases, the oldest python that should be supported is 3.11, so removing support for python < 3.9 is more than reasonable to me. Sphinx 4 is by all accounts quite ancient as well (last release was more than 3 years ago now).

On a personal level, I will note that removing support for older versions of dependencies (and python) has been a huge net improvement on my quality of life when it comes to maintaining pyqtgraph. I was reluctant to pull the trigger, but once I did, it's just been a huge weight off my shoulders.

I'm also of the opinion that SemVer is a lie, so whatever version number you attach to the next release of this extension is fine with me :D

j9ac9k avatar Apr 03 '25 03:04 j9ac9k

and this a good way to start, yeah? I've been using Sphinx for a month now for documentations. I would love to understand more about this project, and I'm determined to learn more.

TriangleBear avatar Apr 03 '25 08:04 TriangleBear

@j9ac9k Good question, should the commit from this PR be cherry-picked into #41, or should this be rebased on a branch with #41 merged?

I think I'm leaning towards cherry-picking this into #41. At this point in time updating CI / testing and chaning away from util.status_iterator needs to happen at the same. Otherwise we'll have a commit where testing is not working or where this is merged untested.

@jonascj would you like me to rebase this PR, I'll be happy to do so.

jonascj avatar Apr 03 '25 20:04 jonascj

@j9ac9k Good question, should the commit from this PR be cherry-picked into #41, or should this be rebased on a branch with #41 merged?

I would do whatever is easier for you, either works, I think at the end of day you need #41 to be green in CI when merging, so whether this commit is the first commit in that branch/PR or this commit is in master and #41 gets rebased off of master will have the same effect (I would imagine cherry-picking would be easiest, and then just close out this PR).

I think I'm leaning towards cherry-picking this into #41. At this point in time updating CI / testing and chaning away from util.status_iterator needs to happen at the same. Otherwise we'll have a commit where testing is not working or where this is merged untested.

@jonascj would you like me to rebase this PR, I'll be happy to do so.

j9ac9k avatar Apr 04 '25 04:04 j9ac9k

@j9ac9k If this PR and #41 did not exist, someone looking at the problem today would have done the equivalent of cherry-picking I belive. Iteratively trying to deal with new versions of tox, python, github actions and sphinx, and likely squashing commits at the end of the messy process.

I'll ask @terencehonles if he can cherry-pick fc159366693c58605eec9c5b45e4a759a6fb9987 from your fork/PR.

jonascj avatar Apr 04 '25 08:04 jonascj

@j9ac9k Thank you for your contribution, it has been incorporated into the project by being cherry picked into #41 : https://github.com/sphinx-contrib/images/commit/8c72db8a978b0c870d3787c99d23634581f4169e

jonascj avatar May 02 '25 22:05 jonascj