Skip to content

Update mypy workflow to include arithmetic_analysis#4288

Closed
dhruvmanila wants to merge 1 commit intoTheAlgorithms:masterfrom
dhruvmanila:update-mypy-workflow
Closed

Update mypy workflow to include arithmetic_analysis#4288
dhruvmanila wants to merge 1 commit intoTheAlgorithms:masterfrom
dhruvmanila:update-mypy-workflow

Conversation

@dhruvmanila
Copy link
Copy Markdown
Member

@dhruvmanila dhruvmanila commented Mar 22, 2021

I didn't receive any error when I ran mypy locally on the directory.

❯ mypy --ignore-missing-imports arithmetic_analysis/
Success: no issues found in 11 source files

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@dhruvmanila dhruvmanila requested a review from cclauss as a code owner March 22, 2021 07:58
@ghost ghost added enhancement This PR modified some existing files awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass labels Mar 22, 2021
@dhruvmanila dhruvmanila marked this pull request as draft March 22, 2021 08:01
@dhruvmanila dhruvmanila removed the request for review from cclauss March 22, 2021 08:03
@cclauss
Copy link
Copy Markdown
Member

cclauss commented Mar 22, 2021

I had the same experience at e578d9d also with the digital_image_processing directory.

@cclauss cclauss self-assigned this Mar 22, 2021
@cclauss cclauss self-requested a review March 22, 2021 08:49
@cclauss
Copy link
Copy Markdown
Member

cclauss commented Mar 22, 2021

@hauntsaninja Do you have any insight on why mypy is passing locally but failing in a GitHub Action job?

@cclauss
Copy link
Copy Markdown
Member

cclauss commented Mar 22, 2021 •
edited
Loading

Perhaps the local tests are passing because numpy is not installed locally and mypy is run with --ignore-missing-imports while numpy is installed in the GitHub Action.

https://numpy.org/devdocs/reference/typing.html

@dhruvmanila
Copy link
Copy Markdown
Member Author

dhruvmanila commented Mar 22, 2021 •
edited
Loading

I did install numpy in my local environment. Even after that it gives me the error. It's mainly because numpy is present that mypy is complaining, if you uninstall numpy, mypy stops complaining. I will investigate further as to why this is.

Edit: For me on both scenario (numpy installed and not installed) mypy does not complain.

@cclauss
Copy link
Copy Markdown
Member

cclauss commented Mar 22, 2021

My sense is that numpy.typing is key to making it pass everywhere.

@cclauss
Copy link
Copy Markdown
Member

cclauss commented Apr 2, 2021

Closed by #4304 ?

@dhruvmanila dhruvmanila closed this Apr 2, 2021
@dhruvmanila dhruvmanila deleted the update-mypy-workflow branch April 3, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cclauss cclauss Awaiting requested review from cclauss cclauss is a code owner

Assignees

@cclauss cclauss

Labels

awaiting reviews This PR is ready to be reviewed enhancement This PR modified some existing files tests are failing Do not merge until tests pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dhruvmanila @cclauss