-
-
Notifications
You must be signed in to change notification settings - Fork 529
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 package included resources reading to standard pkgutil mechanism #53
Conversation
path = expand_resource_path("stopwords/%s.txt" % language) | ||
if not exists(path): | ||
try: | ||
stopwords_data = pkgutil.get_data("sumy", "data/stopwords/%s.txt" % language) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use function expand_resource_path("stopwords/%s.txt" % language)
here so I can change/move folder with resources in single place in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you haven't correctly unsderstood my problem.
In our team we use buildout to develop, build and distribute packages.
Buildout uses egg format of python packages.
In case of egg format expand_resource_path returns paths link '/usr/local/lib/python3.5/site-packages/sumy-0.4.1a0-py3.5.egg/sumy/data/stopwords/english.txt' and it's not a legal file (because of .egg is zip-archive).
For this cases pkgutil ( https://docs.python.org/3.5/library/pkgutil.html#pkgutil.get_data )is single known for me standard way to get access to distributed package data.
Of course I can explicitly add possibility to read in-archive paths for reading data into your code (it you will insist on it) - but I think that usage of standard pkgutil is more pretty way to solve problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or may you have suggested write code the next way?
stopwords_data = pkgutil.get_data("sumy", expand_resource_path("stopwords/%s.txt" % language))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I just wanted to have resources at the single place. But I see function expand_resource_path
is used only at one place in this project, so I think it's OK. Sorry
@heni Thanks for the fix. Please chcek my notes in the code and try to incorporate them and I'll merge :) |
Thanks again. Nice weekend :) |
fix package included resources reading to standard pkgutil mechanism
Old logic doesn't work when I use .egg files to distribute modules because of os.path can't lookup inside .egg archive, but pkgutil can do it!