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

Making snazzier inheritance diagrams #1697

Closed
wants to merge 3 commits into from

Conversation

orbisvicis
Copy link

Three major overhauls:

  • explicit directive arguments will be displayed irrespective of
    the :private-bases: option. This includes builtin classes.
  • Graph nodes can be omitted. For example, given the following
    inheritance chain Exception->TypeError->CustomError and the arguments
    Exception and CustomError, the resulting graph will not display
    TypeError, and the Exception and CustomError nodes will be directly
    linked
  • Nodes of builtin classes have a new style derived from the default
    style. There is also an accompanying option
    inheritance_node_attrs_builtin. As a result, the "style" member
    of the configuration dictionaries is now an iterable.

Todo (maybe):

  • tidy the Graph class
  • provide a new edge style for omitted connections

Three major overhauls:
* explicit directive arguments will be displayed irrespective of
  the :private-bases: option. This includes builtin classes.
* Graph nodes can be omitted. For example, given the following
  inheritance chain Exception->TypeError->CustomError and the arguments
  Exception and CustomError, the resulting graph will not display
  TypeError, and the Exception and CustomError nodes will be directly
  linked
* Nodes of builtin classes have a new style derived from the default
  style. There is also an accompanying option
  inheritance_node_attrs_builtin. As a result, the "style" member
  of the configuration dictionaries is now an iterable.

Todo (maybe):
* tidy the Graph class
* provide a new edge style for omitted connections
@birkenfeld
Copy link
Member

Thanks for the PR! I'll check out the new snazziness when I got a bit of time.

@orbisvicis
Copy link
Author

FTR, the build failure is just a pip-download timeout, unrelated to the actual commit.

@orbisvicis
Copy link
Author

Is there anything that needs to be done before merging ?

@birkenfeld
Copy link
Member

Reviewing right now...

cache.add(node)
yield (name, node)
for name in selector(node):
yield from node_iterate(name, selector)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, "yield from" is not compatible with Python 2.6 and 2.7.

@birkenfeld
Copy link
Member

Graph nodes can be omitted. For example, given the following inheritance chain Exception->TypeError->CustomError and the arguments Exception and CustomError, the resulting graph will not display TypeError, and the Exception and CustomError nodes will be directly linked

Hmm, this doesn't appear to be the case for me.

@property
def names_data(self):
return\
{ name:node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dict comprehensions are not available in Python 2.6.

@lehmannro
Copy link
Contributor

I'm not trying to be picky but could you try to adhere to the general coding style of Sphinx? While it's not explicitly mentioned anywhere it should mostly be PEP 8. (@birkenfeld, correct me if I'm wrong here.)

Especially, parts like this are very unlike all other code we have in Sphinx:

return foo\
  ( bar
  , baz
  )

This would normally expressed just as (assuming you need a line break in there for line length reasons):

return foo(bar,
    baz)

@shimizukawa
Copy link
Member

@orbisvicis @birkenfeld any progress? I'll close this inactive PR at end of Feb 2016.

names = self.roots
cache = set()
selector = lambda v: v.names_out
for name_root in names:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tk0miya
Copy link
Member

tk0miya commented Mar 3, 2016

@orbisvicis @birkenfeld any progress? It's time to close pull request.
I'll close this within a few days if no reactions.

@tk0miya
Copy link
Member

tk0miya commented Mar 6, 2016

Now I close this issue.
Please reopen again if you update pull request.

Thanks,

@tk0miya tk0miya closed this Mar 6, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants