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

Removing the html extra dependency should be considered a breaking change #588

Closed
mdellweg opened this issue Apr 3, 2024 · 4 comments · Fixed by #589
Closed

Removing the html extra dependency should be considered a breaking change #588

mdellweg opened this issue Apr 3, 2024 · 4 comments · Fixed by #589

Comments

@mdellweg
Copy link
Contributor

mdellweg commented Apr 3, 2024

f3ef2e9 introduced a regression by breaking every dependent specifying tablib[html].
Assuming this project does semver (did not find a trace of that in the docs), this should be considered a breaking change unfit for y-releases.
I think the proper way solving this is keeping the "html" optional requirements group and just empty it until the next major release.

@hugovk
Copy link
Member

hugovk commented Apr 3, 2024

So add an empty html = [] extra, and delete it in the next major release?

That sounds reasonable, would you like to open a PR?

mdellweg added a commit to mdellweg/tablib that referenced this issue Apr 3, 2024
Removing such a specifier should be considered a breaking change and be
postponed to the next major release, because `pip install tablib[html]`
fails on this. Since there are no actual dependencies to consider, an
empty list is put behind that key.

fixes jazzband#588
@hugovk
Copy link
Member

hugovk commented Apr 3, 2024

By the way, I get a warning not an error:

pip install "tablib[html]"
Collecting tablib[html]
  Using cached tablib-3.6.0-py3-none-any.whl.metadata (3.7 kB)
WARNING: tablib 3.6.0 does not provide the extra 'html'
Using cached tablib-3.6.0-py3-none-any.whl (47 kB)
Installing collected packages: tablib
Successfully installed tablib-3.6.0

How exactly is it a breaking change? When turning warnings into errors?

I'm still fine with fixing this, but maybe it's less clearly a breaking change.

@mdellweg
Copy link
Contributor Author

mdellweg commented Apr 4, 2024

OK fair, I did assume.
I know we consume it via requireing django-import-export and that fails with pkg_resources.UnknownExtra: tablib 3.6.0 has no such extra feature 'html'.
Let me experiment.

@mdellweg
Copy link
Contributor Author

mdellweg commented Apr 4, 2024

OK, weird:

  • Direct install of tablib[html] issues the warning you mentioned.
  • Consecutive install of django-import-export downgrades tablib to 3.5.0.
  • Direct install of django-import-export selects tablib 3.5.0.

There must be more going on in the dependency solver when more packages are requested. Or the version of pip is just different... But still I'd say the above already fails at the principle of least surprise. And I could swear I once read that one is not supposed to completely remove a optional deps key (cannot find it anymore).

[reproducer; probably not minimal, but 10 out of 10 reproducable] https://github.com/pulp/pulp_ansible/actions/runs/8540040263/job/23396219288?pr=1803#step:10:928

mdellweg added a commit to mdellweg/tablib that referenced this issue Apr 4, 2024
Removing such a specifier should be considered a breaking change and be
postponed to the next major release, because `pip install tablib[html]`
fails on this. Since there are no actual dependencies to consider, an
empty list is put behind that key.

fixes jazzband#588
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 a pull request may close this issue.

2 participants