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

Fix program to work with new dependency versions #426

Closed
wants to merge 3 commits into from

Conversation

bamless
Copy link
Contributor

@bamless bamless commented May 28, 2022

The script was crashing in two places due to changes in behaviour of two dependecies (pillow and slugify).
The crash concerning slugify was caused by a moved __version__ variable, which in turn caused the script to fail during dependency checks.
The crash in PIL was due to a lambda that did not round the returned value to int, thus causing PIL to raise an exception (TypeError float object cannot be interpreted as an integer).

bamless added 2 commits May 28, 2022 19:31
New versions of slugify moved version informations to a standalone
module (__verrsion__.py). Added a check to see if the imported version
is indeed a string (as previously expected). If not, get the
`__version__` variable in the `__version__` module.
@darodi
Copy link
Collaborator

darodi commented May 30, 2022

https://github.com/ciromattia/kcc/blob/master/requirements.txt
https://github.com/ciromattia/kcc/blob/master/setup.py
should be updated too in your pull request for consistency

slugify >= 3.0.0

Duplicate of #411
Duplicate of #410
Duplicate of #417

Pillow >= 8.4.0

Duplicate of #406

@bamless
Copy link
Contributor Author

bamless commented May 30, 2022

Actually, I tested the changes with the versions specified in requirements.txt on a conversion using c2e and they seem to work fine.
Maybe we could leave the old versions as to not cut off platforms without access to the newest ones?
I would further test the code to make sure no other regressions have been introduced, but without unit tests in place its pretty difficult to check every possible execution path. Maybe someone with an in-depth knowledge of the codebase could answer me on that.

@bamless
Copy link
Contributor Author

bamless commented May 30, 2022

Also, I'm noticing some inconsistencies in the versions. For example the requirements file specifies a slugify version >=1.2.1 and <3.0.0, but this info does not match the dependency check inside the code (shared.py::dependencyCheck, >=1.2.1) nor the dependencies listed in setup.py.
Any reasons for that?

@darodi
Copy link
Collaborator

darodi commented May 31, 2022

Also, I'm noticing some inconsistencies in the versions. For example the requirements file specifies a slugify version >=1.2.1 and <3.0.0, but this info does not match the dependency check inside the code (shared.py::dependencyCheck, >=1.2.1) nor the dependencies listed in setup.py. Any reasons for that?

no reason, just an oversight I guess

4b3588d

@darodi
Copy link
Collaborator

darodi commented Jan 14, 2023

merged in #457

@darodi darodi closed this Jan 14, 2023
@darodi
Copy link
Collaborator

darodi commented Jan 14, 2023

@bamless

Thanks a lot!

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