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 #4036 : add markdown format for output of stats script #4037

Merged
merged 15 commits into from
Apr 30, 2019

Conversation

yedhink
Copy link
Contributor

@yedhink yedhink commented Apr 21, 2019

Fixes issue:
fixes #4036

Changes:

  1. Added code for generating output in markdown format
  2. Avoided redundancy by picking out common code as global.
  3. Added argument parsing option to specify the format in which output need, ie:- "txt" or "md" or "all"

To be noted
The markdown file when viewed simply, like a txt file, wouldn't be nicely formatted or anything. Rendering it in browser(say github) or viewing in a markdown viewer would be visually pleasing as promised

How to store output in a file?
python3 scripts/stats.py -f format > location/file_name.format
Example:-
python3 stats.py -f md > ./scripts/STATS.md

@yedhink
Copy link
Contributor Author

yedhink commented Apr 21, 2019

@AdiChat rather than using subheading, the group names looked better as a top level heading, ie:- # HEADING rather than ### HEADING. C.
See if it can be approved too.

Copy link
Member

@arnavb arnavb left a comment

Choose a reason for hiding this comment

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

Doesn't the current markdown provide less information than the previous text file? I don't see any breakdowns for individual algorithms.

@yedhink
Copy link
Contributor Author

yedhink commented Apr 22, 2019

@arnavb @AdiChat fixed the issue with considering only test and src. Previously the script only considered the test and src categories, whereas it should have considered the end categories. Now it has been fixed. See if it can be approved.

Copy link
Member

@arnavb arnavb left a comment

Choose a reason for hiding this comment

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

  1. As a suggestion, since you are using markdown, you could also use Github emojis to indicate whether something is implemented or not. For example, if an algorithm is implemented, you could use :white_check_mark: (:white_check_mark:) for an algorithm which has been implemented. For algorithms not implemented, the dot looks fine.

@yedhink
Copy link
Contributor Author

yedhink commented Apr 23, 2019

@arnavb I like the idea. I think the ✔️ looks better. Have made pr with the change.

Copy link
Member

@arnavb arnavb left a comment

Choose a reason for hiding this comment

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

elm is Elm programming language
exs is Elixir interpreted file

scripts/stats.py Outdated
"sc": "SuperCollider",
"scala": "Scala",
"sh": "Shell Script",
"sml": "Simple Macro Language",
Copy link
Member

Choose a reason for hiding this comment

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

Standard ML

scripts/stats.py Outdated
"rkt": "Racket",
"rs": "Rust",
"ruby": "Ruby",
"sc": "SuperCollider",
Copy link
Member

Choose a reason for hiding this comment

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

The one file with this extension should be a .scala file. Please fix that in this PR. SuperCollider is an audio format if I researched correctly.

"ex": "Euphoria",
"exs": "Elixir",
"f": "Fortran",
"fs": "Visual F#",
Copy link
Member

Choose a reason for hiding this comment

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

Why no Visual for C# but Visual for F# and VB?

scripts/stats.py Outdated
"cpp": "C++",
"cr": "CRiSP",
"cs": "C#",
"elm": "ELM",
Copy link
Member

Choose a reason for hiding this comment

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

Caps?

scripts/stats.py Outdated
"cs": "C#",
"elm": "ELM",
"erl": "Erlang",
"ex": "Euphoria",
Copy link
Member

Choose a reason for hiding this comment

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

Elixir

@yedhink
Copy link
Contributor Author

yedhink commented Apr 28, 2019

@arnavb have made the changes. See if it can be merged.

@arnavb
Copy link
Member

arnavb commented Apr 29, 2019

@AdiChat Do you think this is ready to merge?

@AdiChat
Copy link
Member

AdiChat commented Apr 30, 2019

Yes, this looks good 👍

@AdiChat
Copy link
Member

AdiChat commented Apr 30, 2019

Looks great 😍

Hope you are enjoying your journey 🚆 with OpenGenus
positive_opengenus

@AdiChat AdiChat merged commit 19c1b36 into OpenGenus:master Apr 30, 2019
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.

No markdown format for output of stats script
3 participants