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

Fix the handling of module-level namespaces. #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robottwo
Copy link

@robottwo robottwo commented Jan 3, 2018

Addresses issue #33.

The main part of this patch is to auto-detect the
top level namespace for a given python file. It does
this by walking up the directory tree and scanning for
init files until it locates the top-most one.

This patch also fixes the parsing filter so that it automatically
applies the correct top-level namespace when run on module files
by default. This means no more havingt o put @namespace comments
at the top level of modules. It also suppresses using the @namespace
tag in class definitions, which confuses doxygen into thinking
that classes are namespaces. That's no longer neccessary anyway
because of the module-level namespace.

With this patch, doxypypy produces much better output by default
when run on complex module codebases. There are added options
to suppress the new behavior and get the old behavior back,
for those who don't want namespaces in their documentation
(although I can't imagine why one would want that).

Note that, while it wasn't the original intent, this patch also
makes doxypypy very useful to creating unified documentation
sites -- you can point it at multiple python modules (in a single
doxygen config), and it will create a structured documentation
site combining all of them, organized by namespace. This is a
neat trick for organizations to consolidate internal code
documentation in a single location.

Addresses issue Feneric#33.

The main part of this patch is to auto-detect the
top level namespace for a given python file. It does
this by walking up the directory tree and scanning for
__init__ files until it locates the top-most one.

This patch also fixes the parsing filter so that it automatically
applies the correct top-level namespace when run on module files
by default. This means no more havingt o put @namespace comments
at the top level of modules. It also suppresses using the @namespace
tag in class definitions, which confuses doxygen into thinking
that classes are namespaces. That's no longer neccessary anyway
because of the module-level namespace.

With this patch, doxypypy produces much better output by default
when run on complex module codebases. There are added options
to suppress the new behavior and get the old behavior back,
for those who don't want namespaces in their documentation
(although I can't imagine why one would want that).

Note that, while it wasn't the original intent, this patch also
makes doxypypy very useful to creating unified documentation
sites -- you can point it at multiple python modules (in a single
doxygen config), and it will create a structured documentation
site combining all of them, organized by namespace. This is a
neat trick for organizations to consolidate internal code
documentation in a single location.
@wich
Copy link

wich commented Apr 20, 2018

This is a very desirable feature and works beautifully. Can this please be integrated?

@Feneric
Copy link
Owner

Feneric commented Apr 20, 2018

I'd be happy to add this, but we need some tests for it. All the major features of doxypypy are presently covered by tests in the test folder, and all of these tests get run prior to each release. We don't want to go down the path of adding in new features that'll silently break later, because that won't help anybody.

@buhtz
Copy link

buhtz commented Apr 7, 2022

@robottwo Can you give us a statement about the missing tests please.

@robottwo
Copy link
Author

Sorry, I haven't touched this code in 4+ years. As I recall, it's not clear to me how to write a useful unit test for this particular feature, since the new behavior is a subjective change in the output rendering.

@buhtz
Copy link

buhtz commented Apr 17, 2022

Thanks for your feedback. For me it would be absolute fine if a contributor is not able to know how to write unittests.

@Feneric To honor the work of @robottwo and don't throw it into the trashcan unittests should be written.

One can not presume that each contributor is able to write unittests or even know why this is mandatory.

@Feneric
Copy link
Owner

Feneric commented Jun 26, 2022

@Feneric To honor the work of @robottwo and don't throw it into the trashcan unittests should be written.

You're welcome to write them. My free time is limited, and it's not a huge ask for folks to provide tests that demonstrate that their changes actually work. A lot of people use doxypypy and value its stability. Merging untested changes risks that stability and would potentially break it for other users. I do try to add tests to submitted changes that weren't provided with them over time, but if it wasn't a priority of the submitter to even provide basic certification that their changes work it's not clear to me why it should be higher on my priority list than it was on theirs. My throughput on adding such tests and merging such work is thus quite slow.

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.

4 participants