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

Replace all appearences of pkg_resources except one #101

Merged
merged 7 commits into from
Aug 28, 2023

Conversation

fwitte
Copy link
Member

@fwitte fwitte commented Apr 27, 2023

Ran the tests, all working fine except for the test_examples_datapackages_scripts_infer. There the original implementation already fails locally for me. So I did not touch that.

See #100

@fwitte fwitte requested review from jnnr, gnn and p-snft April 27, 2023 06:17
@fwitte
Copy link
Member Author

fwitte commented Apr 28, 2023

Apparently importlib.resources.files is only available since python 3.9 (https://docs.python.org/3/library/importlib.resources.html).

Copy link
Member

@p-snft p-snft left a comment

Choose a reason for hiding this comment

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

As the changes require Python 3.9, older versions should be removed from both, the the list of supported versions and the testing.

For the failing test, I also have no clue. But I am reluctant to just wave it through.

@fwitte
Copy link
Member Author

fwitte commented May 5, 2023

So we would need to drop support for python 3.8 and lower. Python 3.8 is the default version of ubuntu 20.04 LTS, which will be around for another two years, I guess... I do not really know, how to feel about that. In my opinion it might make sense to move the lower python version limit to 3.9, but that might break things for a couple of people. What do you think?

@fwitte
Copy link
Member Author

fwitte commented May 5, 2023

So, all tests (except the one that did not work in the first place) are working with python 3.9 and 3.10. Python 3.11 has installation issues with cchardet (https://github.com/oemof/oemof-tabular/actions/runs/4891251104/jobs/8731565175).

So the question remains, whether to drop support of older versions. I'll let you guys decide on this, since I do not (yet) use the software...

@p-snft
Copy link
Member

p-snft commented May 5, 2023

Python 3.8 is the default version of ubuntu 20.04 LTS

Honestly, that's the reason why I dist-upgraded.

@henhuy
Copy link
Collaborator

henhuy commented May 9, 2023

I also vote for dropping support for python 3.8

@nailend
Copy link
Collaborator

nailend commented Aug 24, 2023

I also vote for dropping support for python 3.8

nailend
nailend previously approved these changes Aug 24, 2023
@fwitte
Copy link
Member Author

fwitte commented Aug 24, 2023

Anybody has an idea, what the reason for the failing test is?

@nailend
Copy link
Collaborator

nailend commented Aug 24, 2023

Anybody has an idea, what the reason for the failing test is?

which failing tests? I guess this got somehow solved in another issue?!

@fwitte
Copy link
Member Author

fwitte commented Aug 24, 2023

Anybody has an idea, what the reason for the failing test is?

which failing tests? I guess this got somehow solved in another issue?!

... I oversaw it was fixed by merging dev into here 😀

Also, there should be some kind of warning before releasing this into the wild, right? Suddenly disappearing support for 3.8 might be inconvenient.

@nailend
Copy link
Collaborator

nailend commented Aug 24, 2023

Also, there should be some kind of warning before releasing this into the wild, right? Suddenly disappearing support for 3.8 might be inconvenient.

Yes, that's right. I'm not sure how many active users there are, though. Following the release convention, we haven't even released a feature, only bug fixes :D, so there is no stable version yet. v0.0.3

How is this normally handled? You would raise a warning in another release? Or is it maybe fine to set a tag before and mention it in the warning?

@nailend nailend dismissed their stale review August 24, 2023 16:37

This should not be merged without setting a tag before, due to the dropped support of pyton3.8

Copy link
Collaborator

@nailend nailend left a comment

Choose a reason for hiding this comment

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

A warning for dropped support of python 3.8 should be added and possibly the option to use a version/tag which still supports 3.8

tests/test_examples.py Outdated Show resolved Hide resolved
tests/test_examples.py Outdated Show resolved Hide resolved
@nailend nailend merged commit bd8b2f6 into dev Aug 28, 2023
2 checks passed
@nailend nailend deleted the fix/#100-pgk_resources-depriciation branch August 28, 2023 13:22
@fwitte
Copy link
Member Author

fwitte commented Aug 29, 2023

@nailend: since this has been merged as well there is no more point in the warning message :D. People with python < 3.9 will not be able to install the latest version anymore. I would suggest downgrade the python requirement again, make a release, upgrade it again and make another release 2 or so weeks later.

@nailend nailend restored the fix/#100-pgk_resources-depriciation branch August 30, 2023 11:01
@nailend nailend linked an issue Aug 30, 2023 that may be closed by this pull request
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.

pkg_resources is deprecated
4 participants