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

Add flake8 and spell check, whitelist dictionary, docstrings #155

Merged
merged 4 commits into from
Dec 29, 2020

Conversation

asears
Copy link
Contributor

@asears asears commented Dec 26, 2020

Description

In learning more about opt_einsum and its codebase and dependencies, would like to take some time improve on the documentation by implementing flake8-spellcheck and fixing some minor spelling and markdown format issues.

Some items addressed in PR:

  1. Add flake8 and spell check packages to conda full test.
  2. Add a whitelist dictionary
  3. Fix docstring spelling SC100 issues
  4. Fix markdown formatting

Todos

Add Terminology / Glossary to documentation.

Questions

  1. Does the change to Conda environment files break any CI builds?
  2. Does the change to readme file break any Sphinx documentation formatting?

Status

  • Ready to go

@codecov
Copy link

codecov bot commented Dec 26, 2020

Codecov Report

Merging #155 (82f430d) into master (32fa384) will not change coverage.
The diff coverage is 100.00%.

Copy link
Owner

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

Overall LGTM, thanks for changes.

Can you also add a small note in opt_einsum/RELEASE.md to signify that we should run spellcheck over the codebase before a release?

README.md Show resolved Hide resolved
opt_einsum/contract.py Outdated Show resolved Hide resolved
whitelist.txt Outdated Show resolved Hide resolved
…ntract

Signed-off-by: asears <asears@users.noreply.github.com>
Signed-off-by: asears <asears@users.noreply.github.com>
Copy link
Owner

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

LGTM, I'll merge after CI passes. Thanks again for the changes!

@dgasmith dgasmith merged commit c6ff814 into dgasmith:master Dec 29, 2020
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.

None yet

2 participants