search

szarroug3 / X-Ray_Calibre_Plugin

X-Ray Creator plugin for Calibre
http://www.mobileread.com/forums/showthread.php?t=273189
GNU General Public License v3.0
57 stars 12 forks source link

Merge code for auto-guessing aliases #18

Closed stoduk closed 8 years ago

stoduk commented 8 years ago

https://github.com/stoduk/calibre_plugin_xray/blob/master/xrayaction.py - see fullname_to_possible_aliases and auto_expand_aliases

I added this functionality to the other plugin, so that when a character name was looked up it would guess some aliases. eg. "Mr Ebenezer Jimmy Scrooge" would be aliased to "Mr Scrooge", "Ebenezer", "Ebenezer Scrooge", etc. But if there was "Bob Cratchet" and "Tim Cratchet" then "Cratchet" wouldn't be an alias for either, as it wasn't unique (though "Bob" and "Tim" aliases could still be created).

It improved matches drastically for very little effort.

I assume it makes sense to add this to this plugin, so just a question of where/how/whether this is a config option or default behaviour.

First guess: it could be controlled by a config flag (so people can turn off it if they don't want it), and can be done whenever the "Book specific preferences" option is populating the alias list for the first time.

Discuss :)

stoduk commented 8 years ago

@szarroug3 - thoughts on this?

szarroug3 commented 8 years ago

@stoduk I love this idea. I was looking for a way to do it but could never figure out how to do it when I had people such as Elend Venture and Lord Venture who weren't the same people.

I definitely agree that this should be optional. I think most people would prefer the option to be on rather than off so the default state of the option in the preferences can be on.

I agree that adding it when the aliases is populating the first makes the most sense. Remember, though, that that happens in 2 places (which I don't like but couldn't figure out how to get it to use just the one place to do without causing problems.. I can look into cleaning that up)

Side note: I created 2.1.1 zip yesterday but I completely forgot to add you as an author. Can you add yourself on your next commit? Sorry about that!

stoduk commented 8 years ago

While I think of it, are you bumping the version numbers in the code? I thought I saw 1.0.0 or similar when I last installed (but could be wrong!)

On 18 May 2016, at 14:10, szarroug3 notifications@github.com wrote:

@stoduk I love this idea. I was looking for a way to do it but could never figure out how to do it when I had people such as Elend Venture and Lord Venture who weren't the same people.

I definitely agree that this should be optional. I think most people would prefer the option to be on rather than off so the default state of the option in the preferences can be on.

I agree that adding it when the aliases is populating the first makes the most sense. Remember, though, that that happens in 2 places (which I don't like but couldn't figure out how to get it to use just the one place to do without causing problems.. I can look into cleaning that up)

Side note: I created 2.1.1 zip yesterday but I completely forgot to add you as an author. Can you add yourself on your next commit? Sorry about that!

ā€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

szarroug3 commented 8 years ago

LOL nice catch, I updated to 1.1.0 but stopped there... :(

szarroug3 commented 8 years ago

@stoduk did you already start working on this? i started and i have one portion of it done.. i forgot to assign it to myself when I started :disappointed:

stoduk commented 8 years ago

Yeah, but luckily not the biggest bit of work. Probably a good idea to always have an assigned bug when doing something so we don't duplicate work.

Talking of which - are you working on start/end actions? We spoke about it previously, and you said you had a partial diff I think.

Cheers, Anthony

On 25 May 2016 at 15:26, szarroug3 notifications@github.com wrote:

@stoduk https://github.com/stoduk did you already start working on this? i started and i have one portion of it done.. i forgot to assign it to myself when is started šŸ˜ž

ā€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/szarroug3/X-Ray_Calibre_Plugin/issues/18#issuecomment-221593113

szarroug3 commented 8 years ago

Yeah sorry about that. I just started working on author profiles earlier today. I was mostly researching on how to do it then I randomly started working on it. I did some research on the progress thing and got nowhere.

On Wed, May 25, 2016, 4:27 PM Anthony Toole notifications@github.com wrote:

Yeah, but luckily not the biggest bit of work. Probably a good idea to always have an assigned bug when doing something so we don't duplicate work.

Talking of which - are you working on start/end actions? We spoke about it previously, and you said you had a partial diff I think.

Cheers, Anthony

On 25 May 2016 at 15:26, szarroug3 notifications@github.com wrote:

@stoduk https://github.com/stoduk did you already start working on this? i started and i have one portion of it done.. i forgot to assign it to myself when is started šŸ˜ž

ā€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub < https://github.com/szarroug3/X-Ray_Calibre_Plugin/issues/18#issuecomment-221593113

ā€” You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub https://github.com/szarroug3/X-Ray_Calibre_Plugin/issues/18#issuecomment-221712835

stoduk commented 8 years ago

I just looked at the code you committed, while answering your comment on the other issue, and spotted a problem.

You flipped the check 'title in "Lord"' to be ''lord' in title' - both will work, but the former is more right. Technically it is missing square brackets for clarity. It was meant to be 'title in ["Lord"]' - and I was going to flesh out the list of honorifics for which is is valid to use with a Christian name. A quick google hasn't really clarified the situation - some argue that "Mr Anthony" is valid English, others argue it is not. I imagine "Lord Anthony" would be more acceptable to most, while Sgt Major Anthony would not (christian name with a military title seems wrong).

I'll raise a bug to track this, even if we don't yet know what the fix is. The common answer is "grammar follows use" - so perhaps we can just start with some list of honorifics that use christian names and if people find books that use other honorifics we can add them.

Do you know of any other changes you made to the code during the C&P?

On 26 May 2016 at 05:01, Samreen Zarroug notifications@github.com wrote:

Yeah sorry about that. I just started working on author profiles earlier today. I was mostly researching on how to do it then I randomly started working on it. I did some research on the progress thing and got nowhere.

On Wed, May 25, 2016, 4:27 PM Anthony Toole notifications@github.com wrote:

Yeah, but luckily not the biggest bit of work. Probably a good idea to always have an assigned bug when doing something so we don't duplicate work.

Talking of which - are you working on start/end actions? We spoke about it previously, and you said you had a partial diff I think.

Cheers, Anthony

On 25 May 2016 at 15:26, szarroug3 notifications@github.com wrote:

@stoduk https://github.com/stoduk did you already start working on this? i started and i have one portion of it done.. i forgot to assign it to myself when is started šŸ˜ž

ā€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub <

https://github.com/szarroug3/X-Ray_Calibre_Plugin/issues/18#issuecomment-221593113

ā€” You are receiving this because you modified the open/close state.

Reply to this email directly or view it on GitHub < https://github.com/szarroug3/X-Ray_Calibre_Plugin/issues/18#issuecomment-221712835

ā€” You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/szarroug3/X-Ray_Calibre_Plugin/issues/18#issuecomment-221771985

szarroug3 commented 8 years ago

I totally misunderstood that. I assigned myself the issue you created. As for other changes, yeah I had to change the auto_expand_aliases a lot because it seems like you were doing things very differently but I think the way I did it essentially does the same thing yours was doing for your code. The only thing I didn't do was the AKA parsing. I think that needs to be done in ShelfariParser. I'll create an issue for it.