option to read numbers as individual digits

[nvaccessAuto]

Reported by blindbhavya on 2014-07-12 11:50 Lets take a big number for example 20000. I want the number to be read as 2 0 0 0 0. A user may want an option to read a number as an individual digit due to several reasons. Like a sighted person converts the visible numbers into thousands and millions a blind person may also want to test himself. Also, telephone numbers by no reason should be pronounced as one number. So there should be an option somewhere that would allow the user to do this. Blocking #178, #4779, #4872

[nvaccessAuto]

Attachment dev.nvda-addon added by driemer.riemer@... on 2014-07-25 21:23 Description: Prototype addon giving this support.

[nvaccessAuto]

Comment 1 by jteh on 2014-07-13 01:30 A couple of notes:

1. For now, this can already be done using speech dictionary rules, though you have to have a reasonable knowledge of regular expressions to manage this.
2. We used to have a rule to do this automatically for telephone numbers. Unfortunately, telephone numbers aren't formatted consistently, so it was taking effect for numbers where it shouldn't. Consequently, it was removed.

[nvaccessAuto]

Comment 2 by driemer.riemer@... on 2014-07-13 05:12 What would need done to implement this? I could do it, as I have a good knowledge of regular expressions. I wouldn't mind adding a checkbox that does this to the document formatting or voice page. This could maybe edit the speech dictionarries to achieve this?

[nvaccessAuto]

Comment 3 by blindbhavya on 2014-07-13 06:05 Hi. I think most people who encounter telephone numbers would not know about regular expressions. just a simple check box in Speech Dictionaries or Document Formtting or Punctuation Symbol Pronunciation dialog.

[nvaccessAuto]

Comment 4 by jteh on 2014-07-13 09:24 My first note was just pointing out that it can be done already if you need it. The request is still valid. (Otherwise, I would have closed the ticket.)

As to telephone numbers, as I explained in my second note, they aren't formatted consistently, so there's no reliable way to do this. It's either speaking all numbers as separate digits or all numbers as full numbers.

[nvaccessAuto]

Comment 5 by blindbhavya on 2014-07-13 10:11 Hi. Yes, so maybe there could be a combo box to choose whether to read numbers as individual digits or to read as whole numbers. Could you implement this? It could be put in the Punctuation Symbol Pronunciation dialog box?

[nvaccessAuto]

Comment 6 by driemer.riemer@... on 2014-07-13 19:10 I am going to try to first implement this in an addon, and then if it works take steps to incorperate it into core code. I will start this as soon as I finish my work on audacity support.

[nvaccessAuto]

Comment 7 by blindbhavya on 2014-07-14 11:56 Hi. Thanks for taking this responsibility. If possible, do send me a private e-mail when the add-on is ready for testing. I am on the NVDA add-ons list though.

[nvaccessAuto]

Comment 8 by driemer.riemer@... on 2014-07-25 21:23 This gives initial support of this functionality.

[nvaccessAuto]

Comment 9 by blindbhavya on 2014-07-26 04:48 Hi. Thanks for this. However, I feel that this addon should be incorporated into NVDA itself.

[nvaccessAuto]

Comment 10 by driemer.riemer@... on 2014-07-26 12:34 There are a few technical things I have questions about before I incorperate it into core. Mainly, is it safe to use the temp dict or should I use builtin, and a few other things. I would like it to be more transparent than having a new temperary dictionary created. I will work on that next week.

[nvaccessAuto]

Comment 11 by blindbhavya on 2014-07-26 13:41 Ok.

[nvaccessAuto]

Comment 13 by jteh on 2014-07-28 06:42 I've marked #178 as a duplicate, but note that #178 suggests an option to read as double or triple digits as well as single digits.

I think the builtin dictionary is most appropriate, but we need to make sure that the entry can be removed easily when the configuration option is disabled.

[nvaccessAuto]

Comment 14 by driemer.riemer@... on 2014-07-28 16:51 working with builtin should be the same. This works by mapping the dictionary name to a list of SpeechDictEntry instances. The only problem is where to put this in the code, and/or if it would be better managed as a speech api function. I need to do more research about how I could implement this efficiently, such that nvda wouldn't start to get bloated with random features that get requested over the years. Any ideas on ways to implement this or if simply putting my addon somewhere in the code and changing to builtin would suffice?

[nvaccessAuto]

Comment 15 by jteh on 2014-08-05 10:24 On further thought, for a core implementation, I don't think we should bother putting this in a dictionary at all. Instead, I think it makes sense to just check the configuration option in speechDictHandler.processText and do the replacement (using a constant regular expression) if the option is enabled. This is a lot simpler to manage.

[nvaccessAuto]

Comment 16 by blindbhavya on 2014-08-05 12:01 Hi. Thank you to both of you for getting the implementation of this feature under process. Thank you for this! Hope it will be in 2014.3!!!

[nvaccessAuto]

Comment 17 by driemer.riemer@... on 2014-08-10 03:08 Hey, I got the numbers to work right. Try this build and tell me if it was what you were looking for. Also, is it okay to submit a pull request, or would you guys rather have a patch? click here to download the try build for t4273.

[nvaccessAuto]

Comment 18 by driemer.riemer@... on 2014-08-10 03:11 remember to uninstall the digits or numbers addon, as it won't be necessary with this build. That was my first implementation.

[nvaccessAuto]

Comment 19 by beqa on 2014-08-10 03:16 yes, it works without problems

[nvaccessAuto]

Comment 20 by driemer.riemer@... on 2014-08-10 03:20 Cool! I was hoping so. So now, do I or does nvaccess usually put in the whats new entry?

[nvaccessAuto]

Comment 21 by nvdakor on 2014-08-10 07:17 Hi, Do you have a public Git repository, based your work from master and did your modifications in a separate branch? If so, let us know the URL for your repo and the branch name so Mick and Jamie can incorporate your branch. Alternatively, someone can commit your changes as part of a new branch called t4273 which will be stored on NV Access's server and tag it for code review for you. Once your patch is accepted, it'll be incubated for two weeks to see if there are bug reports on it. Once there are no commits in two weeks, then Mick and/or Jamie will incorporate it into master, at which point changes entry will be written. One more thing: in additoin to code changes, did you write an entry in the user guide explaining what your new option is? This is needed so users will know what to do (as for milestones and keywords, let's wait until Mick and adds them). Thanks.

[nvaccessAuto]

Comment 22 by driemer.riemer@... (in reply to comment 21) on 2014-08-11 02:29 Okay. I submitted a pull request, but my computer was screwing with me, so I think that I might have accidentally submitted a patch for another ticket that I did no work on. Let me know if a t4273 branch was created if you except the patch. My git repo is derek's git repo overview page. I am also happy to generate a .patch file if need be. You might want to mark this so you know it still needs a whats new entry. Replying to nvdakor:

Hi,

Do you have a public Git repository, based your work from master and did your modifications in a separate branch? If so, let us know the URL for your repo and the branch name so Mick and Jamie can incorporate your branch. Alternatively, someone can commit your changes as part of a new branch called t4273 which will be stored on NV Access's server and tag it for code review for you.

Once your patch is accepted, it'll be incubated for two weeks to see if there are bug reports on it. Once there are no commits in two weeks, then Mick and/or Jamie will incorporate it into master, at which point changes entry will be written.

One more thing: in additoin to code changes, did you write an entry in the user guide explaining what your new option is? This is needed so users will know what to do (as for milestones and keywords, let's wait until Mick and adds them).

Thanks.

[nvaccessAuto]

Comment 23 by jutzi1 on 2014-08-27 17:16 As suggested, here are the steps Emailed to me for modifying the default speech dictionary to pronounce numbers as digits. I back up my dictionary so I don't need to do this whenever I reinstall Windows and NVDA.

following strings will be in quotes but when putting it in they should be without quotes. Pattern: "(\d)(?=\d)" Replacement: "\1 " (note the trailing space at the end; this is very important) Comment: "Numbers as single digits" Check the Regular expression checkbox.

[nvaccessAuto]

Comment 24 by jteh on 2014-08-28 03:23 I'm assuming the correct URL is https://bitbucket.org/derek_riemer/nvda? I don't see a t4273 branch in that repo.

[nvaccessAuto]

Comment 25 by beqa on 2014-08-28 04:34 hi.

no, the correct url is https://bitbucket.org/derek_riemer_/nvda

[nvaccessAuto]

Comment 26 by driemer.riemer@... on 2014-08-28 05:27 That is correct. The derekriemer one. The other one is my other eamil account's bitbucket.

[nvaccessAuto]

Comment 27 by blindbhavya on 2014-08-28 09:51 Hello. One of the primary situations in which this feature would be very useful is while reading telephone numbers. These could pop up any time, so I think a shortcut should be there to quickly toggle between reading individual digits or numbers. I am not sure what keystroke that would be though. What about NVDA Ctrl N? n is the initial letter of numbers so would be easy to remember.

[nvaccessAuto]

Comment 28 by jteh on 2014-08-29 06:27 This looks pretty good. Nice work.

A few comments:

Some people have requested the ability to speak numbers as small groups of digits (e.g. 123456 could be 12 34 56 or 123 456) as well as single digits. Is this something people think could actually be useful? If so, we should incorporate it into this feature now, rather than having to introduce a new setting later.

+++ b/source/speechDictHandler.py
@@ -90,6 +90,8 @@ def processText(text):
...
      text=dictionaries[+ if config.conf["speech"](type].sub(text)
)["speakDigits"]:

I wonder whether this should happen before we process other dictionaries instead of after, since it's built in. However, you could argue it's better to do it after so that users can override this in specific cases. Thoughts?

+     text=re.sub(r"(\d)(?=\d)", r"\1 ", text)

This should be compiled and placed in a constant outside of (probably just above) the function, as this avoids the need to recompile it every time it is used, which is much more efficient. For example:

RE_NUMBERS_DIGITS = re.compile(r"(\d)(?=\d)")
...
        text=RE_NUMBERS_DIGITS.sub(r"\1 ",text)

+++ b/user_docs/en/userGuide.t2t
@@ -698,6 +698,9 @@ Usually, NVDA raises the pitch slightly for any capital letter, but some synthes
...
+This checkbox allows you to toggle whether nvda will speak numbers as words, or digits. For example, if this is checked nvda will speak the number "2482" as "2 4 8 2". 

nit: These two sentences should be on separate lines. (We do one sentence per line in the User Guide to make life easier for translators.) Also, remove the comma before the "or".

+++ b/source/config/__init__.py
@@ -73,6 +73,7 @@ confspec = ConfigObj(StringIO(
...
+ speakDigits=boolean(default=false)

nit: There should be a space on either side of the equals sign after speakDigits, just as there is for the other options. I know the NVDA code is pretty inconsistent as far as spaces around operators go, but it's best to be consistent with the area you're working in.

[nvaccessAuto]

Comment 29 by jteh on 2014-08-29 06:30 Btw, Derek, in case you didn't know, you can associate multiple email addresses (and ssh keys) with the same Bitbucket account, rather than needing separate accounts.

[nvaccessAuto]

Comment 30 by k_kolev1985 (in reply to comment 28) on 2014-08-29 06:43 Replying to jteh:

Some people have requested the ability to speak numbers as small groups of digits (e.g. 123456 could be 12 34 56 or 123 456) as well as single digits. Is this something people think could actually be useful? If so, we should incorporate it into this feature now, rather than having to introduce a new setting later.

Yes, I for one would prefer to have a more flexible option about how the digits are read: one by one, in pairs, in triplets.

[nvaccessAuto]

Comment 31 by driemer.riemer@... on 2014-08-29 08:44 Ah. Cool. The main reason I created a new account, isI forgot the password for the other one that I need for sinking, and in order to change it, I need the old one. Btw, I can try and create regexps for the other types of number modes, but phone numbers and such might be a bit tough. There are a lot of edge cases. I guess the advantage here is that we can do more than regexps, because we are in code here. I would assume that we want to keep processText kind of simple though since it is probably important for performance. (thinking about the things we are getting ready to talk about in my systems class this semester). I might have a bit of time this weekend to add support to this.

[nvaccessAuto]

Comment 32 by jteh on 2014-08-29 09:42 Don't try to support phone numbers specifically. Phone numbers are formatted and spoken differently across countries. We've been down this path before and it was a big flop. Just focus on single digits, double digits and triple digits I think. You'll probably have to cache the compiled regular expression when the number is changed.

Performance is most definitely very important for processText, since it processes every string that is spoken.

[nvaccessAuto]

Comment 33 by driemer.riemer@... on 2014-08-29 23:15 Okay. I did a bit of research on this. It looks like, in order to split numbers in groups of 3 logically, we need to somehow split the string up with 1 to 3 3's at the beginning of the string, and then groups of 3's afterwords. This is a bit tricky with regular expressions, because the regular expression tries to be "greety" and if you disable that, it doesn't. This is problematic because a number like 3333333 should say 3 333 333 not 333 333 3 which the regular expression (\d{3}) does even if you tell it to try and match 1 to 3 chars. My solutions to this problem are as follows.

1. Make a simple reversal of the string, apply this regexp to it and then reverse the result. Example, 1234567 would become 7654321 then the regexp is applied making it turn into "765 432 1" and then after a reversal it would be "1 234 567"
2. Find the length of a number group and then figure out if it is divisible by 3 evenly. If so, then apply the regexp (\d{3}(?=\d{3}) to this string. elifit has a remainder of 1, then place a space at position 1 (0 based) of the string of number's, and then apply the previous regexp to this new string that has the rest of the number's. elif it has a remainder of 2, then place a space at position 2 (0 based) of the string of number's, and then apply the previous regexp to this new string that has the rest of the number's.

The greatness of this is it would work for 2 based or n based number groups as well with this second one. I don't like that first option, because it is going to get preportionally harder to reverse the string as the string grows in size, and then that reversal happens twice. The second one requires that processText would only receive number's or that we figure out where number groups start and end in the string. Also we have the challenge that strings are immutable, so we can't insert a space at position n without copping the string (I think). So any other idea's/comments?

[nvaccessAuto]

Comment 34 by driemer.riemer@... on 2014-08-29 23:15 On the shortcut key, how do I do this?

[nvaccessAuto]

Comment 35 by jteh on 2014-08-30 09:00 I really think a shortcut key is overkill for this. You can easily spell the current word if you need it temporarily.

[nvaccessAuto]

Comment 36 by driemer.riemer@... on 2014-08-30 19:14 Any comments on comment 33?

[nvaccessAuto]

Comment 37 by zahari_bgr on 2014-08-30 19:56 Hi, try this: re.sub(r'(\d)(?=(\d{3})+$)', r'\1 ', text) I got the regexp from the first comment here and it works on my python installation: http://stackoverflow.com/questions/721304/insert-commas-into-number-string

[nvaccessAuto]

Comment 38 by driemer.riemer@... on 2014-08-31 17:08 Okay. I am putting this here since it seems to be the magic regexp. I have optimized it and it seems to work. (\d{1,3})(?=(\d{3})+(\D|\b)) For now, can people who are watching this ticket put that regexp in there speech dictionaries with a replacement of "\1 " without the quotes, and make sure to include the space!!! I am gonna start coding it now, and if it seems to not give people problems, I will recommend it be set for code review. I am gonna try it with a combo box to select either single digits, double digits, tripple digits, or read numbers as words. The default will still be "words" People can optionally have double digits read by doing (\d{1,2})(?=(\d{2})+(\D|\b))

[nvaccessAuto]

Comment 39 by driemer.riemer@... on 2014-09-02 16:24 Hi, The changes are made to t4273 on my git repo. https://bitbucket.org/derek_riemer_/nvda

[nvaccessAuto]

Comment 40 by jteh on 2014-09-03 10:09 Thanks. Looks pretty good. Code review:

+++ b/source/config/__init__.py
...
+digitChoices=[```
I understand that you want to keep config stuff together. However, these strings are only needed for the UI, so I think they should be kept with the GUI code. You could either make the list a class attribute or just include it as part of the call.

• Translators: Choice in a combo box for speaking numbers by word.

• "words",

* IMO, "words" could be misleading, as technically, digits are words. Perhaps "full numbers" is better.
* You've correctly included a translators comment, since this is a translatable string. However, you need to mark it with _() for it to be translatable.

• "Tripple Digits"](

)

* Triple is spelled with a single p, not a double p.
* Capitalise just the first word for multiple word choices.

@@ -73,6 +85,7 @@ confspec = ConfigObj(StringIO(

...

• speakDigits=integer(default=0,min=0,max=3)

nit: Please put spaces around the = sign.

+++ b/source/gui/settingsDialogs.py

...

• digitsLabel=wx.StaticText(self, wx.IDANY, label=("Speak numbers as"))

* nit: Please place a colon after the label for consistency with other combo boxes.
* Add an accelerator, perhaps the n. Just put an & before it; e.g. "&numbers".

+++ b/source/speechDictHandler.py

...

+#compiled regular expressions for reading digits.

+digits = re.compile(r"(\d)(?=\d)")

* General convention is for constants to be upper case, although I realise some of our code doesn't do this. Also, it'd be good if the name was clearer about the fact that it's a regexp; e.g. RE_DIGITS.
* I'd probably put these in a dict matching the config option choices; e.g.

RE_NUMBERS = { 1: re.compile(r"(\d)(?=\d)"), ... }

 This way, you can just look it up in the dict later; see below.

@@ -90,6 +94,13 @@ def processText(text):

...

• if digitConf==1:

• text=re.sub(digits, r"\1 ", text)

• If you use the dict idea above, you can just do: if digitConf > 0: text = RE_NUMBERS[digitConf].sub(r"\1 ", text)
• Also note that you can call the .sub method on a compiled regexp rather than using re.sub.

Good work!

[nvaccessAuto]

Comment 41 by driemer.riemer@... on 2014-09-06 00:31 Hi all,

== Lets take a vote. ==

please tell me if you want this to happen before the speech dictionaries are processed, or after. If it happens before, any specific modifications to numbers will be lost, so I would say afterwords.

Also, Jamie,

Do you want me to modify that first return statement? Currently, the way this is implemented, we have a return statement if dictionaries are turned off, which will completely block this feature from working. This should probably still work if the user has speech dictionaries off, since it is a separate config setting.

[nvaccessAuto]

Comment 42 by driemer.riemer@... on 2014-09-07 22:13 For now I am going to route around dictionary processing, so that if dictionaries are on, we process them, but if they are off, we don't process them, but process numbers.

[nvaccessAuto]

Comment 43 by jteh on 2014-09-07 22:15 For now, let's go with:

• Processing numbers afterwards; and
• Processing numbers even if dictionaries are turned off.

[nvaccessAuto]

Comment 44 by driemer.riemer@... on 2014-09-07 22:46 Okay. i will work on that when I have time.

[nvaccessAuto]

Comment 45 by driemer.riemer@... on 2014-09-14 04:38 Done! try pulling my changes in t4273.

[nvaccessAuto]

Comment 46 by driemer.riemer@... on 2014-09-14 09:34 Actually, please wait till I can test more cause I am having compilation issues.

[nvaccessAuto]

Comment 48 by driemer.riemer@... on 2014-09-15 16:09 You might not want to do code review yet, because I had an error with scons, and couldn't test this yet. I will let you know once I can get it tested.

[nvaccessAuto]

Comment 49 by jteh on 2014-10-08 02:46 Dropping needsCodeReview as per comment:48. Please let us know when this is ready for review. Thanks.

[nvaccessAuto]

Comment 50 by driemer.riemer@... on 2014-12-22 04:07 So as you might know my whole git repo is so screwed I am scrapping the whold branch and starting over.I should have this done soonish.

[nvaccessAuto]

Comment 51 by driemer.riemer@... on 2014-12-22 04:14 Hey nvaccess, would It bug you if I change the 2007 in the header of speechDictHandler.py to 2015 now? It is currently set to 2007 and we are close enough to 2015 anyway..