[Draft] Integrate the MathCAT add-on into NVDA #18323

RyanMcCleary · 2025-06-25T00:40:17Z

Link to issue number:

Summary of the issue:

The MathCAT add-on is not currently included in the core of NVDA. Integrating MathCAT into NVDA will give users access to mathematical content out of the box.

Description of user facing changes:

The core functionality of the add-on will remain the same, but MathCAT will be included without needing to install it as an add-on (replacing mathPlayer). An additional panel will be added in the NVDA preferences dialog containing the configuration that is currently handled in MathCAT's add-on configuration dialog. Additionally, the user docs will be updated to reflect the replacement of mathPlayer with MathCAT.

Description of developer facing changes:

The code from the MathCAT add-on has been refactored and cleaned up to conform to NVDA's coding standards, and that code is included in the mathPres/MathCAT directory with this PR. Dependencies have been added as a Git submodule in include/nvda-mathcat. Before this PR is ready for review, changes will also be made to the way MathCAT is initialized (currently, it is only set up to be loaded as an add-on), and YAML configuration will be replaced with configobj.

Description of development approach:

Developments will proceed in the following steps:

Update the initialization of MathCAT so that it is loading and running correctly in its current form.
Include MathCAT configuration into the NVDA preferences dialog.
Replace YAML configuration with configobj.
Add unit and system tests.

Testing strategy:

The add-on code after refactoring has been tested only by loading it into NVDA and testing it functionally. Discussion of more unit and system tests is ongoing.

Known issues with pull request:

Previously, libmathcat_py.pyd was included in that MathCAT directory. Now, it is located in include/assets, so it is not being imported correctly. This can be fixed by updating pyproject.toml with the new path.

Code Review Checklist:

Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
API is compatible with existing add-ons.
Security precautions taken.

@coderabbitai summary

josephsl · 2025-06-25T00:52:00Z

Hi, I think we discussed this, but if it does replace MathPlayer, can that be documented in the user guide or the math presentation section be rewritten to use MathCAT content? Thanks.

AppVeyorBot · 2025-06-25T03:07:05Z

CI timing (mins):
INIT 0.0,
INSTALL_START 1.6,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 20.1,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 20.5,
FINISH_END 0.2
PASS: Unit tests.
PASS: Translation comments check.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/6yki35ou7nee78pw/artifacts/output/nvda_snapshot_pr18323-36962,28d9c669.exe
PASS: License check.
FAIL: System tests (tags: installer NVDA). See test results for more information.

See test results for failed build of commit 28d9c66976

…to mathcat-integration Merging

…endencies

…to mathcat-integration Merging

AppVeyorBot · 2025-06-26T21:14:15Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 4298ae7b2f

AppVeyorBot · 2025-06-27T03:40:59Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 7445b8eba5

AppVeyorBot · 2025-06-27T19:04:48Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 9427fd7aa8

AppVeyorBot · 2025-06-27T20:29:37Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 4ba7a139a0

AppVeyorBot · 2025-06-27T21:45:43Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 27b2f891ff

AppVeyorBot · 2025-06-30T21:51:52Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 0368c286bf

AppVeyorBot · 2025-07-01T05:26:49Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 111a571a00

SaschaCowley · 2025-07-02T02:06:02Z

pyproject.toml

@@ -47,6 +47,7 @@ dependencies = [
 	"mdx_truly_sane_lists==1.3",
 	"markdown-link-attr-modifier==0.2.1",
 	"mdx-gh-links==0.4",
+	"pyyaml==6.0.2",


Hate to butt in on a draft PR, but can we consider using StrictYAML instead? It sidesteps several of the rather nasty issues with YAML while being mostly compatible with PyYAML.

(I'd suggest not using YAML at all, but I suspect that's rather out of scope)

I think that would require upstream changes to https://github.com/nsoiffer/MathCAT/, which we don't intended on maintaining. We're just parsing rules for math designed in YAML files as set by Neil. Since we don't have control over the source yaml, I don't think we can change our parsing rules.

seanbudd · 2025-07-02T01:47:08Z

.gitmodules

+[submodule "source/mathPres/MathCAT/nvda-mathcat"]
+	path = source/mathPres/MathCAT/nvda-mathcat
+	url = https://github.com/ryanmccleary/nvda-mathcat
+[submodule "include/nvda-mathcat"]
+	path = include/nvda-mathcat
+	url = https://github.com/ryanmccleary/nvda-mathcat


can you update these to just a single submodule referencing https://github.com/nvaccess/nvda-mathcat/

seanbudd · 2025-07-02T01:47:36Z

pyproject.toml


 [tool.uv.workspace]
 members = [
    "miscDeps",
+	"include/nvda-mathcat",


Suggested change

"include/nvda-mathcat",

"include/nvda-mathcat",

seanbudd · 2025-07-02T01:53:28Z

source/mathPres/MathCAT/MathCATPreferences.py

+		return os.path.join(os.path.dirname(os.path.abspath(__file__)), "Rules", "Braille")
+
+	@staticmethod
+	def languagesDict() -> dict[str, str]:


This function should instead return a set of supported languages for MathCAT, and the language names should be derived from getLanguageDescription.
Any language code which is not supported by getLanguageDescription should get added to LANG_NAMES_TO_LOCALIZED_DESCS.

seanbudd · 2025-07-02T02:22:33Z

pyproject.toml

@@ -47,6 +47,7 @@ dependencies = [
 	"mdx_truly_sane_lists==1.3",
 	"markdown-link-attr-modifier==0.2.1",
 	"mdx-gh-links==0.4",
+	"pyyaml==6.0.2",


I think that would require upstream changes to https://github.com/nsoiffer/MathCAT/, which we don't intended on maintaining. We're just parsing rules for math designed in YAML files as set by Neil. Since we don't have control over the source yaml, I don't think we can change our parsing rules.

RyanMcCleary · 2025-07-02T03:45:32Z

Hi Joseph, Thanks for bringing this up. We can update the user docs with information on MathCAT instead of mathPlayer. I'll add a note about that to this PR.

…

On Tue, Jun 24, 2025 at 8:52 PM Joseph Lee ***@***.***> wrote: *josephsl* left a comment (nvaccess/nvda#18323) <#18323 (comment)> Hi, I think we discussed this, but if it does replace MathPlayer, can that be documented in the user guide or the math presentation section be rewritten to use MathCAT content? Thanks. — Reply to this email directly, view it on GitHub <#18323 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFYRGABPBI2BH3SISCZ3L33FHXENAVCNFSM6AAAAACABZTVP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMBSGI3DIOJVG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

AppVeyorBot · 2025-07-02T06:55:15Z

Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit f7b9c5fce2

RyanMcCleary added 4 commits June 23, 2025 18:00

Added MathCAT addon code

4955ac8

Added MathCAT dependencies as submodule.

51fc43f

Removed mathPres/MathCAT/nvda-mathcat submodule

cbbdcf0

Moved nvda-mathcat submodule into include

bed4c94

RyanMcCleary requested a review from a team as a code owner June 25, 2025 00:40

RyanMcCleary requested a review from seanbudd June 25, 2025 00:40

Pre-commit auto-fix

a99b1a6

RyanMcCleary marked this pull request as draft June 25, 2025 00:45

RyanMcCleary and others added 6 commits June 26, 2025 15:48

Added mathcat-assets to data_files

81cf1ec

Merge branch 'mathcat-integration' of github.com:RyanMcCleary/nvda in…

993f0a9

…to mathcat-integration Merging

Added mathcat assets directory to extraPaths in pyproject.toml

0291fd3

Pre-commit auto-fix

6d2d502

Added nvda-mathcat to tool.uv.sources, tool.uv.workspace, and dev dep…

90b11d2

…endencies

Merge branch 'mathcat-integration' of github.com:RyanMcCleary/nvda in…

b3bde52

…to mathcat-integration Merging

Fixed errors in pyproject.toml

a1ba905

RyanMcCleary added 3 commits June 27, 2025 13:32

Fixed type annotation error in MathCAT.py

c09ee27

Fixed libmathcat_py import in MathCAT.py

97485c4

Fixed broken ProsodyCommand import

cbb578e

RyanMcCleary and others added 4 commits June 27, 2025 15:18

Removed use of addonHandler in MathCAT

c72aec9

Pre-commit auto-fix

78cf34a

Fixed MathCAT initialization

8ca47de

Fixed merge conflict

b085645

RyanMcCleary and others added 2 commits June 27, 2025 16:42

Fixed Rules path

13e358d

Pre-commit auto-fix

a4ec06b

Adjusted rules path

f69332d

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jul 1, 2025

adjusted rules dir path for launcher build

a266836

RyanMcCleary added 3 commits July 1, 2025 20:49

Removed markdown files

bf7bb2c

Added pyyaml dependency

1912c4c

Removed yaml subdirectory of source/mathPres/MathCAT

28c0969

SaschaCowley reviewed Jul 2, 2025

View reviewed changes

Fixed yaml import

6e9d94d

seanbudd reviewed Jul 2, 2025

View reviewed changes

RyanMcCleary and others added 2 commits July 1, 2025 23:16

Removed use of addonHandler in MathCATgui and MathCATPreferences

db46c15

Pre-commit auto-fix

4047976

Uh oh!

[Draft] Integrate the MathCAT add-on into NVDA #18323

Are you sure you want to change the base?

[Draft] Integrate the MathCAT add-on into NVDA #18323

Conversation

RyanMcCleary commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

josephsl commented Jun 25, 2025 via email

Uh oh!

AppVeyorBot commented Jun 25, 2025

Uh oh!

AppVeyorBot commented Jun 26, 2025

Uh oh!

AppVeyorBot commented Jun 27, 2025

Uh oh!

AppVeyorBot commented Jun 27, 2025

Uh oh!

AppVeyorBot commented Jun 27, 2025

Uh oh!

AppVeyorBot commented Jun 27, 2025

Uh oh!

AppVeyorBot commented Jun 30, 2025

Uh oh!

AppVeyorBot commented Jul 1, 2025

Uh oh!

SaschaCowley Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

seanbudd Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

RyanMcCleary commented Jul 2, 2025 via email

Uh oh!

AppVeyorBot commented Jul 2, 2025

Uh oh!

Uh oh!

RyanMcCleary commented Jun 25, 2025 •

edited

Loading