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

Added -a flag to include all words in acronym #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added -a flag to include all words in acronym #6

wants to merge 3 commits into from

Conversation

ygrange
Copy link

@ygrange ygrange commented Apr 1, 2019

This pull request adds an option to the tool that makes sure each acronym returned will contain letters from every word in the description. I figured that writing a feature request wouldn't take much shorter than implementing it and pull-requesting.

The only thing I am not really sure about is how to treat words like "a", "the", "in", etc. which in general would be ignored.

@Morkney
Copy link

Morkney commented Apr 2, 2019

Hi,

This is a great addition but can result in some possible acronyms being missed. This is because the base code will only return one combination for each acronym even when there are many available. If it coincidentally returns an alternative combination that doesn't use one letter per word, then that particular acronym will not be shown.

For example:

> acronym --all-words "engineering dwarfs galaxy edge"
Collecting word corpus
Identifying matching acronyms
Process Complete
ENGAGED	ENGineering dwArfs Galaxy EDge
ENRAGED	ENgineeRing dwArfs Galaxy EDge
EAGLE	Engineering dwArfs GaLaxy edgE
EDGED	Engineering Dwarfs Galaxy EDge

... which misses the acronym "EDGE".

@ygrange
Copy link
Author

ygrange commented Apr 4, 2019

I went into the code thinking that this wouldn't be too complex to fix, but I think it would be a bit more work thatn I naively assumed :)

Probably at cost of performance.
@ygrange
Copy link
Author

ygrange commented Apr 25, 2019

Ok. New version, this one does find EDGE for example. I basically use a completely different search strategy. I haven't really looked at performance (if relevant at all anyway).

@ygrange
Copy link
Author

ygrange commented Apr 25, 2019

This BTW adds a dependency to itertools. If I am not mistaken that is part of the default python libraries anyway.

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