Skip to content

Conversation

orgh0
Copy link
Collaborator

@orgh0 orgh0 commented Jul 12, 2018

@sushain97 Docstring isolated PR

@coveralls
Copy link

Pull Request Test Coverage Report for Build 182

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 80.441%

Totals Coverage Status
Change from base Build 159: 0.0%
Covered Lines: 584
Relevant Lines: 726

💛 - Coveralls

2 similar comments
@coveralls
Copy link

Pull Request Test Coverage Report for Build 182

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 80.441%

Totals Coverage Status
Change from base Build 159: 0.0%
Covered Lines: 584
Relevant Lines: 726

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build 182

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 80.441%

Totals Coverage Status
Change from base Build 159: 0.0%
Covered Lines: 584
Relevant Lines: 726

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jul 12, 2018

Pull Request Test Coverage Report for Build 183

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 80.414%

Totals Coverage Status
Change from base Build 159: -0.03%
Covered Lines: 583
Relevant Lines: 725

💛 - Coveralls

@sushain97
Copy link
Member

This is a good start. The docs should look more like this:

https://github.com/apertium/streamparser/blob/master/streamparser.py#L187

So that they can be used to generate a pretty HTML page.

@orgh0
Copy link
Collaborator Author

orgh0 commented Jul 16, 2018

@sushain97 Alright I'll change all the docstrings to that format !

@orgh0
Copy link
Collaborator Author

orgh0 commented Jul 24, 2018

@sushain97 I've updated the docstrings. I'll cover more functions if required. How do these look?
Suggestions for changes?

Copy link
Member

@sushain97 sushain97 left a comment

Choose a reason for hiding this comment

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

I looked through some of this but there are a ton of spelling mistakes that should be fixed.

def update_modes(pair_path): # type: (str) -> None
modes = search_path(pair_path)
def update_modes(path): # type: (str) -> None
"""Updates the install modes
Copy link
Member

Choose a reason for hiding this comment

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

install => installed

Args:
path(str): A string that is the absolute location to the modes to be installed
Yelids:
Copy link
Member

Choose a reason for hiding this comment

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

Yelids => Yields

This won't be picked up by the parser.

Moreover, it should probably be returns.

Actually, does it even return anything?

"""Updates the install modes
Args:
path(str): A string that is the absolute location to the modes to be installed
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't make it clear that this path will be added, the old ones won't be removed.



class Analyzer:
"""An Analyzer object containing it's analysis mode and langugage
Copy link
Member

Choose a reason for hiding this comment

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

it's => its

Copy link
Member

Choose a reason for hiding this comment

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

langugage => language

"""Updates the install modes
Args:
path(str): A string that is the absolute location to the modes to be installed
Copy link
Member

Choose a reason for hiding this comment

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

path(str) => path (str)

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.

3 participants