issue #113, create markdown with jinja template in status view

[zhou-pj]

Description

Motivation and Context

see issue #113

Types of Changes

• Documentation update
• Bug fix
• New feature
• Breaking change¹

¹The change breaks (or has the potential to break) existing functionality.

Checklist:

• I am familiar with the Contributing Guidelines.
• I agree with the terms of the Contributor Agreement.
• My name is on the list of contributors.
• My code follows the code style guideline of this project.
• The changes introduced by this pull request are covered by existing or newly introduced tests.

If necessary:

• I have updated the API documentation as part of the package doc-strings.
• I have created a separate pull request to update the framework documentation on signac-docs and linked it here.
• I have updated the changelog.

[zhou-pj]

NOT ready for review yet, but would like to discuss the proper markdown rendering package and how to proceed at some point. I have modified all the template to create roughly the needed markdown syntax, will need further improvements upon figuring out the rendering method.

[vyasr]

@zhou-pj just for future reference, Github supports Draft PRs that allow you to make a pull request and ask for feedback while indicating that it's not yet ready for a thorough review and merge. Next time when you open a PR, click the arrow next to the green button for creating the PR and you'll see that one of the options is creating a draft PR.

[b-butler]

I really like the new terminal markdown render's view. I also think the raw view is now sufficient ugly that we should require mdv by default. However, the html output could be better. Having the detailed status have the fields more separated, would be easier to parse. In addition, using markdown headers for detailed and overview and things like that would help guide the eye more. One more thing job operation submission status is aligned in the terminal but not the html. I think them both being aligned would be best.

[vyasr]

@zhou-pj what was the final result of the various renderers we discussed? Were you able to get HTML to render in the terminal, or was the markdown viewer package the best option? I agree with @b-butler that we definitely need something to prettify the default markdown output for the terminal. Can you summarize the conclusions you/we came to on the various package options for this rendering (mistune, misteletoe, mdv, markdown, html2text, etc)?

[zhou-pj]

Sorry for the delay... @vyasr @b-butler To summarize, I think the first step: using jinjia template to generate markdown status output looks pretty good to me now. For second step rendering on terminal, I get terminal_markdown_viewer to work if we generate terminal view directly from markdown (but I need to avoid using any '|' in output as this is used for markdown table formatting). I tried the html2text to see if html to terminal works but it looks like this package basically convert html back to markdown, not for terminal rendering. And as for providing html version for user, I think mistune works well to create html. I'll push a cleaned commit later today using mistune and terminal_markdown_viewer.

[vyasr]

I'm currently a bit swamped trying to finalize the 2.0 release of freud as well as get some data collection done for a project. I'll look back into this as soon as possible, but it'll probably be another week or so. Sorry for the delay @zhou-pj, I'll get to this as soon as possible.

[bdice]

@zhou-pj Here's a potentially helpful method for removing the ANSI escape sequences use re.sub. The regex is taken from: https://stackoverflow.com/questions/14693701/how-can-i-remove-the-ansi-escape-sequences-from-a-string-in-python

>>> ansi_string = '\033[1mbold\033[0m text'
>>> print(len(ansi_string))
17
>>> ansi_escape = r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])'
>>> plain_string = re.sub(ansi_escape, '', ansi_string)
>>> print(len(plain_string))
9

[bdice]

I left some comments. I haven't closed followed this PR yet so I wanted to familiarize myself with it. It looks good overall! Thanks @zhou-pj for the good work.

[csadorf]

There are currently 4 reviewers on this PR, that seems a bit overkill. I'm happy to look at this, but maybe we can reduce that a bit?

[bdice]

There are currently 4 reviewers on this PR, that seems a bit overkill. I'm happy to look at this, but maybe we can reduce that a bit?

I only read over this PR because I wanted to know how the new system worked. I am going to step back and let other reviewers help finalize this PR, I just wanted a bit of familiarity with what's being done and suggested changes while I was reading through it.

[csadorf]

There are currently 4 reviewers on this PR, that seems a bit overkill. I'm happy to look at this, but maybe we can reduce that a bit?

I only read over this PR because I wanted to know how the new system worked. I am going to step back and let other reviewers help finalize this PR, I just wanted a bit of familiarity with what's being done and suggested changes while I was reading through it.

That's totally fine and makes a lot of sense. I just wanted to make sure that we don't "burden" too many folks with the review requests.

@b-butler I'll remove the request for you for now, but please feel free to review this PR if you feel so inclined.

[zhou-pj]

@zhou-pj Here's a potentially helpful method for removing the ANSI escape sequences use re.sub. The regex is taken from: https://stackoverflow.com/questions/14693701/how-can-i-remove-the-ansi-escape-sequences-from-a-string-in-python

>>> ansi_string = '\033[1mbold\033[0m text'
>>> print(len(ansi_string))
17
>>> ansi_escape = r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])'
>>> plain_string = re.sub(ansi_escape, '', ansi_string)
>>> print(len(plain_string))
9

Thanks for the suggestions! It turn out the tabulate.py has this implemented and the real issue was the way I add terminal support to mistune make used tabulate.py before the bold markup(\033[1m) is applied, so it cased that problem. I modified in mistune to fix it.

[csadorf]

This is fantastic! I only have a comments and questions.

[csadorf]

@vyasr LGTM, please merge at your discretion.

@zhou-pj Great work!!

[vyasr]

I have a few small inline comments. In addition, please update the API documentation to include the new Renderer class, and the return type of print_status. You'll also need to update the Changelog. Also, we'll probably want to include a small update to signac-docs about the existence of the Renderer in the Topic Guides section on status printing. We can merge this PR without that one being created, but it would be good to have on your radar (maybe make an issue if you don't have time to start on that immediately so that it's at least recorded that we want to add the documentation).

[vyasr]

Thanks for pushing this through! Sorry it took so long.