isac322 commented 3 years ago

Pull Request Checklist

Thank you for taking the time to improve Arrow! Before submitting your pull request, please check all appropriate boxes:

[ ] 🧪 Added tests for changed code.
[x] 🛠️ All tests pass when run locally (run tox or make test to find out!).
[x] 🧹 All linting checks pass when run locally (run tox -e lint or make lint to find out!).
[ ] 📚 Updated documentation for changed code.
[x] ⏩ Code is up-to-date with the master branch.

If you have any questions about your code changes or any of the points above, please submit your questions along with the pull request and we will try our best to help!

Description of Changes

Added type annotation to all functions and methods.
Add runtime dependency typing_extensions for backward compatibility with 3.6 and 3.7
Add mypy git pre-commit hook
Add mypy configuration file (setup.cfg)
Refactor some logics for better type annotation
- I will create comments for these

Please review this with my comments on PR. I have questions.

codecov[bot] commented 3 years ago

Codecov Report

Merging #883 (6aba5ed) into master (f2e0849) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #883   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines         1817      1886   +69     
  Branches       318       311    -7     
=========================================
+ Hits          1817      1886   +69

Impacted Files	Coverage Δ
arrow/api.py	`100.00% <100.00%> (ø)`
arrow/arrow.py	`100.00% <100.00%> (ø)`
arrow/constants.py	`100.00% <100.00%> (ø)`
arrow/factory.py	`100.00% <100.00%> (ø)`
arrow/formatter.py	`100.00% <100.00%> (ø)`
arrow/locales.py	`100.00% <100.00%> (ø)`
arrow/parser.py	`100.00% <100.00%> (ø)`
arrow/util.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f2e0849...6aba5ed. Read the comment docs.

nkitsaini commented 3 years ago

I am excited for type hint in arrow. Is there anything I can do to help this get merged (faster) in master ?

jadchaar commented 3 years ago

Hi @nkitsaini i have been a bit busy lately, but I should be more free to finish reviewing this in the next couple of weeks. We are hoping to ship this as part of v1.0 by the end of the year!

nkitsaini commented 3 years ago

Thanks for quick reply (and sorry for my late reply). No worries actually, end of the year is far better then what I was expecting.

jadchaar commented 3 years ago

Hey @isac322 I should be able to review this in the next couple of weeks as things wind down for me. Mind getting this branch up to date with master?

slavaatsig commented 3 years ago

Why not support PEP-563?

jadchaar commented 3 years ago

Why not support PEP-563?

Doesn't PEP-563 only support Python 3.7+? We must support 3.6+.

jadchaar commented 3 years ago

Hey @isac322, any progress on addressing feedback? We really appreciate your work here, and would love to get this merged soon! This is the last big puzzle piece for version 1.0 :).

isac322 commented 3 years ago

@jadchaar Sorry for late reply. I'm currently quite busy for my marriage and company reorganization. Can I check this on last week of January?

jadchaar commented 3 years ago

@jadchaar Sorry for late reply. I'm currently quite busy for my marriage and company reorganization. Can I check this on last week of January?

No problem at all buddy, take your time! Also, congratulations and best of luck with the marriage/reorganization :).

isac322 commented 3 years ago

@jadchaar @systemcatch Could you please mark conversation as resolved if you satisfied on changes for future work? Because there are many conversation, it's quite hard to distinguish TODOs 😢.

systemcatch commented 3 years ago

I've gone through and marked many conversations as resolved, the few remaining ones need Jad's input.

jadchaar commented 3 years ago

I've gone through and marked many conversations as resolved, the few remaining ones need Jad's input.

Fantastic and thanks for the hard work @isac322, we really do appreciate it. I will give this a good final look either today or tomorrow!

isac322 commented 3 years ago

@jadchaar @systemcatch It was a long journey, but it was fun. Thanks for your work!

maroux commented 3 years ago

This is missing py.typed support which means tools like mypy can't check arrow usage. I created #925 for that.

arrow-py / arrow

Add type annotations #883

Pull Request Checklist

Description of Changes

Codecov Report