Removing the html extra dependency should be considered a breaking change

jazzband / tablib

Python Module for Tabular Datasets in XLS, CSV, JSON, YAML, &c.

https://tablib.readthedocs.io/

MIT License

4.6k stars 592 forks source link

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

Closed mdellweg closed 6 months ago

mdellweg commented 6 months ago

f3ef2e94b40478521b646b58a25dd1b3c5b300f1 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 commented 6 months ago

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

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

hugovk commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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