Diaoul / babelfish

BabelFish is a Python library to work with countries and languages
BSD 3-Clause "New" or "Revised" License
25 stars 13 forks source link

Use stevedore as Extension Manager #13

Closed Toilal closed 10 years ago

Toilal commented 10 years ago

As discussed for guessit, babelfish should use stevedore as extension manager.

It works for unit tests, and embedded in guessit too (unit tests passing)

Ready for 0.5 ?

Diaoul commented 10 years ago

Thanks

Diaoul commented 10 years ago

After a few testing I saw that stevedore calls load on the EntryPoint object which is OK when you have deployed your library but doesn't work without running setup.py. Loading this way makes pkg_resources call require on the entry point which then uses self.dist.requires which is None because EntryPoint.parse didn't set it. And this fail here.

`NoneType` has no attribute 'requires'

This is too bad because there are no extras to require and self.dist.requires would've ended up doing nothing but since it's called anyway, it crashes.

Instead of fixing this in stevedore I did something simple on my own based on your design and pushed it to a new branch called converter_manager. There are a few backward incompatible changes but that's for the sake of simplicity. There's no need to run setup.py you can use babelfish right away (given that it's in your PYTHONPATH of course).

I'd like your feedback on this please @Toilal and @wackou

wackou commented 10 years ago

I'm out of town tomorrow and for the weekend but I'll have a look at it first thing next week.

On Thu, Jan 23, 2014 at 11:32 PM, Antoine Bertin notifications@github.comwrote:

After a few testing I saw that stevedore calls load on the EntryPointobject which is OK when you have deployed your library but doesn't work without running setup.py. Loading this way makes pkg_resources call require on the entry point which then uses self.dist.requires which is None because EntryPoint.parsedidn't set it. And this fail here.

NoneType has no attribute 'requires'

This is too bad because there are no extras to require and self.dist.requires would've ended up doing nothing but since it's called anyway, it crashes.

Instead of fixing this in stevedore I did something simple on my own based on your design and pushed it to a new branch called converter_manager. There are a few backward incompatible changes but that's for the sake of simplicity. There's no need to run setup.py you can use babelfish right away (given that it's in your PYTHONPATH of course).

I'd like your feedback on this please @Toilal https://github.com/Toilaland @wackou https://github.com/wackou

Reply to this email directly or view it on GitHubhttps://github.com/Diaoul/babelfish/pull/13#issuecomment-33176856 .

Toilal commented 10 years ago

@Diaoul I have handled the problem you speak in guessit by overiding _load_one_plugin and checking is ep.dist is set. @see L70 transformers.py.

I'll try the same fix in the master branch by using stevedore. In the converter_manager, the register_custom_converter fails, i'll have a look too.

Toilal commented 10 years ago

Ok, i've fix both branches. register_custom_converter test was failing because a converter object must be passed to the manager, but the test passed a converter class.

I think the converter_manager branch (without stevedore) is the way to go for babelfish !

Toilal commented 10 years ago

And it's working properly with custom converter in guessit, @see guessit babelfish-dev branch.

I let you move the converter_manager branch to master, and then release babelfish :+1: