sphinx-contrib / matlabdomain

A Sphinx extension for documenting Matlab code
Other
64 stars 45 forks source link

Subfolders in class folders are inappropriately treated as class folders. #214

Closed BuildingAtom closed 8 months ago

BuildingAtom commented 11 months ago

It seems like subfolders in class folders are processed as if they are MATLAB class folder modules. The assert error is thrown by line 246 of mat_types.py since subfolders inside of class folders can't be class folders. The case for the private subfolder is written explicitly here, but it implies this for all subfolders. The parent folder of a class folder must be in a path folder and a class folder is not itself considered a path folder.

An simple example of where this is a problem is when private methods are used in class folder modules. Those are expected to reside inside of the private subfolder, which causes this assert error to be thrown as a MatClass won't be found inside that subfolder.

For example, if we have a class folder ./@MyClass

./@MyClass
  |__ MyClass.m
  |__ public_function.m
  \__ private/
      |__ private_function.m
      \__ other_private_function.m

This is parsed into the class_folder_modules dictionary:

{
    '@MyClass': <MatModule: "@MyClass">,
    '@MyClass.private': <MatModule: "private">,
}

with class_folder_modules['@MyClass'].entities containing:

[
    ('MyClass', <MatClass: "MyClass">),
    ('public_function', <MatMethod: "public_function">),
]

and class_folder_modules['@MyClass.private'].entities containing the following (no MatClass):

[
    ('private_function', <MatMethod: "private_function">),
    ('other_private_function', <MatMethod: "other_private_function">),
]

The same issue occurs if there is a private folder holding non-matlab code (such as c source files for MEX-ing).

joeced commented 11 months ago

Hi.

Thanks for the very detailed report. I assume this is with the latest update in 0.20.0.

I'll take a look at it ver soon

joeced commented 11 months ago

I released a new version, 0.20.1, please verify that the issue is fixed.

BuildingAtom commented 11 months ago

Thanks! It appears to be fixed for the private subfolder case, but it still throws the same assert for any general subfolder. This might be a non-standard use, but I use a subfolder of a class folder to also store C++ code that would get mex'd during runtime and that src subfolder still triggers the same assertion.

joeced commented 11 months ago

Ah okay. Understood. So, we should ignore any subfolder below a @folder?

BuildingAtom commented 11 months ago

If I understand MATLAB's documentation correctly, I think so. Let me check MATLAB lets you create another class in a class folder.

BuildingAtom commented 11 months ago

image In this example, I was only able to create tempclass and tempclass.class4.

When I attempt to force add the path for .../@tempclass using addpath, MATLAB gives the following warning:

Warning: Method directories not allowed in MATLAB path: [...]/scripts/@tempclass 
> In path (line 109)
In addpath (line 86) 

I can't run tempclass.class2 - MATLAB autocorrects it to tempclass.class4, and using the Run button yields the same behavior. When I try tempclass.class3 through the Run button or command window, MATLAB prompts me to add [...]/scripts/@tempclass to the path, which fails.

The same behavior for tempclass.class3 happens for class5 and MATLAB doesn't let me add the [...]/scripts/@tempclass/aaa folder to the path.

So at the very least, it seems that MATLAB R2022b doesn't support any classes in any subfolders of class folders.

joeced commented 11 months ago

I released a new version, 0.20.2, please verify that the issue is fixed now... And again, thanks for the very detailed reports and follow-up. Really appreciated 👍

BuildingAtom commented 11 months ago

No problem, thanks for the super useful tool!

It looks like it's working correctly now. It throws the assertion error for the above testclass since it finds 2, but that's expected because a class folder should only have 1 class. Otherwise, everything seems to be working!

joeced commented 10 months ago

I'll keep this issue open as I've seen similar parsing issues with other class-folder constructs:

Class-folder without class definition

@ClassFolder
    random_file.m
joeced commented 8 months ago

Release 0.21.0 fixes the rest of the known issues.