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

Command Line Interface Update #91

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Command Line Interface Update #91

wants to merge 12 commits into from

Conversation

nguyenmail
Copy link
Collaborator

@nguyenmail nguyenmail commented Apr 26, 2021

A command-line interface feature has been added. This allows user to run the summary and word frequency function on the terminal and save the results on a .json file. The user can choose whether they want to save the results or have them printed on the terminal, the files will be kept in a separate folder.

What is the current behavior?

#34

What is the new behavior if this PR is merged?

Users should be able to run textmining.py, where the CLI is contained.

Type of change

Please describe the pull request as one of the following:

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

Other information

This PR has:

  • Commit messages that are correctly formatted
  • Tests for newly introduced code
  • Docstrings for newly introduced code

Developers

@nguyenmail and @cookaa

cookaa
cookaa approved these changes Apr 26, 2021
@jjumadinova
Copy link
Contributor

Please ensure your branch is up to date with the master branch.

@munzekm munzekm self-requested a review April 28, 2021 18:57
@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #91 (1d114d6) into master (244b0ba) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #91   +/-   ##
=======================================
  Coverage   92.09%   92.09%           
=======================================
  Files           6        6           
  Lines         253      253           
=======================================
  Hits          233      233           
  Misses         20       20           

@kemandel kemandel added the Team 1 Team 1 is working on this issue label Apr 28, 2021
Copy link
Contributor

@enpuyou enpuyou left a comment

Choose a reason for hiding this comment

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

An overall comment, can you give us an example of what the JSON files would look like for the summary and frequencies? Thank you!

if record:
# Write the summary into a json file.
data = sz.summarizer(directory)
file = open(path + "/" + record + ".json", "w")
Copy link
Contributor

Choose a reason for hiding this comment

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

us os.path.sep instead of /

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team 1 Team 1 is working on this issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants