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

Create markdown representation #986

Merged
merged 19 commits into from
Dec 25, 2021
Merged

Create markdown representation #986

merged 19 commits into from
Dec 25, 2021

Conversation

dellaert
Copy link
Member

This PR adds markdown formatting for discrete factors and conditionals. Main new functions are

  • DiscreteConditional::_repr_markdown_
  • DecisionTreeFactor::_repr_markdown_

These method names are not very c++ like but work in python notebooks as is. Waiting for a PR in wrap so we can rename them to "markdown", in analogy to "dot". @varunagrawal :-)

An example:

image

Key formatters are supported but need a bit more machinery on python side, e.g.,

class MD:
    """Small class that renders markdown with optional arguments."""
    def __init__(self, obj, *args):
        self.md = obj._repr_markdown_(*args)
    def _repr_markdown_(self):
        return self.md

@dellaert dellaert added feature New proposed feature quick-review Quick and easy PR to review wrapper Related to the wrapper labels Dec 24, 2021
@varunagrawal
Copy link
Collaborator

Should I only do markdown for now and add other methods later? Or is there some subset of functions that you would find immediately useful.

@dellaert
Copy link
Member Author

Should I only do markdown for now and add other methods later? Or is there some subset of functions that you would find immediately useful.

It's just a loop over strings, right? Just add all supported formats, including mime.

Copy link
Collaborator

@varunagrawal varunagrawal left a comment

Choose a reason for hiding this comment

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

Straightforward PR.


// Find the MPE
for(DiscreteValues& frontalVals: allPosbValues) {
for(const auto& frontalVals: allPosbValues) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor comment: auto&& will automatically take care of constness.

varunagrawal and others added 5 commits December 25, 2021 09:30
767a74718 Merge pull request #142 from borglab/python/repr-methods
1cbbd7757 make the repr method generation much simpler
b154ed0ba add support for special ipython methods and do some refactoring
f0f72283d update test fixtures

git-subtree-dir: wrap
git-subtree-split: 767a74718e25aa9fa8831e99ce7c459f216043cf
@dellaert
Copy link
Member Author

Cool, thanks for joint effort! I will merge after CI passes.

@dellaert dellaert merged commit 501a6db into develop Dec 25, 2021
@dellaert dellaert deleted the feature/markdown branch December 25, 2021 18:01
@varunagrawal
Copy link
Collaborator

Woohoo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New proposed feature quick-review Quick and easy PR to review wrapper Related to the wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants