PyElastica icon indicating copy to clipboard operation
PyElastica copied to clipboard

Add some more typehinting to files at project root

Open ankith26 opened this issue 1 year ago • 1 comments

This PR does some initial work on https://github.com/GazzolaLab/PyElastica/issues/255

This is a WIP, and I've opened this PR to get some feedback, before I go ahead and look into more modules.

ankith26 avatar Mar 09 '24 17:03 ankith26

@ankith26 I allowed the github-action to run for you to check. For formatting, you can use make formatting or setup-up a pre-commit hook.

skim0119 avatar Mar 09 '24 20:03 skim0119

@armantekinalp Since we are bumping up to 3.10, lets see if we can pass this PR and add mypy checker.

skim0119 avatar Apr 25 '24 22:04 skim0119

@ankith26 if you are still interested in this issue can you resolve the conflicts please.

armantekinalp avatar Apr 26 '24 01:04 armantekinalp

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (update/mypy@f3c650e). Click here to learn what that means.

:exclamation: Current head 4ec414a differs from pull request most recent head 034d62b. Consider uploading reports for the commit 034d62b to get more accurate results

Additional details and impacted files
@@              Coverage Diff               @@
##             update/mypy     #341   +/-   ##
==============================================
  Coverage               ?   87.80%           
==============================================
  Files                  ?       43           
  Lines                  ?     2943           
  Branches               ?      341           
==============================================
  Hits                   ?     2584           
  Misses                 ?      335           
  Partials               ?       24           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 26 '24 05:04 codecov[bot]

I have force pushed with the conflicts fixed. The conflicts were mainly due to the fact that I was adding type hints to the functions that were being deprecated in the update-0.3.3 branch. I removed my changes in these cases

ankith26 avatar Apr 26 '24 05:04 ankith26

@ankith26 thanks. Can you also see why the CI builds fail?

armantekinalp avatar Apr 26 '24 16:04 armantekinalp

While resolving the conflicts I forgot to remove an import that is now unneeded. I have removed that import to fix the fail, and also made sure make autoflake8-check passed locally

ankith26 avatar Apr 26 '24 16:04 ankith26

@ankith26 Hi -- I added mypy on GitHub action, and, as we have somewhat expected, it is becoming a large job to finish. We have decided that we should split the work on #255, and we can probably pass this PR without completely checking out all CIs.

Thank you again for your contribution. If you can finish typehinting for elastica/rod directory, we'll merge this PR with #367. You can run mypy --config-file pyproject.toml elastica/rod

skim0119 avatar Apr 28 '24 19:04 skim0119