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

Enhance nativescript.py type hint #129

Merged
merged 7 commits into from Nov 13, 2022

Conversation

daehan-koreapool
Copy link
Contributor

Goal

  • Enhance nativescript module type hint to improve code predictability and maintainability
poetry run flake8 pycardano
poetry run mypy --install-types --non-interactive pycardano
pycardano/nativescript.py:38: error: Value of type "Union[bytes, bytearray, str, int, float, Decimal, bool, None, Tuple[Any, ...], List[Any], IndefiniteList, Dict[Any, Any], defaultdict[Any, Any], OrderedDict[Any, Any], Any, datetime, Pattern[Any], Any, Any, Set[Any], FrozenSet[Any]]" is not indexable  [index]
pycardano/nativescript.py:47: error: "type" has no attribute "_TYPE"  [attr-defined]
pycardano/nativescript.py:48: error: Argument 2 for "super" not an instance of argument 1  [misc]
pycardano/nativescript.py:48: error: Value of type "Union[bytes, bytearray, str, int, float, Decimal, bool, None, Tuple[Any, ...], List[Any], IndefiniteList, Dict[Any, Any], defaultdict[Any, Any], OrderedDict[Any, Any], Any, datetime, Pattern[Any], Any, Any, Set[Any], FrozenSet[Any]]" is not indexable  [index]
pycardano/nativescript.py:55: error: Unsupported operand types for + ("bytes" and "str")  [operator]
pycardano/nativescript.py:55: note: Right operand is of type "Union[str, bytes]"
pycardano/nativescript.py:68: error: "type" has no attribute "json_tag"  [attr-defined]
pycardano/nativescript.py:81: error: Argument 2 for "super" not an instance of argument 1  [misc]
pycardano/nativescript.py:90: error: "type" has no attribute "json_tag"  [attr-defined]
pycardano/nativescript.py:102: error: "type" has no attribute "_TYPE"  [attr-defined]
pycardano/nativescript.py:130: error: Incompatible types in assignment (expression has type "List[Any]", target has type "str")  [assignment]
pycardano/nativescript.py:134: error: Incompatible types in assignment (expression has type "int", target has type "str")  [assignment]
pycardano/nativescript.py:212: error: Incompatible types in assignment (expression has type "None", variable has type "int")  [assignment]
pycardano/nativescript.py:221: error: Incompatible types in assignment (expression has type "None", variable has type "int")  [assignment]
Found 13 errors in 1 file (checked 12 source files)
make: *** [Makefile:65: qa] Error 1

@daehan-koreapool daehan-koreapool added the enhancement New feature or request label Nov 12, 2022
@daehan-koreapool daehan-koreapool self-assigned this Nov 12, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 12, 2022

Codecov Report

Merging #129 (7faa94b) into main (c5ef3bc) will increase coverage by 0.04%.
The diff coverage is 96.29%.

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   85.95%   85.99%   +0.04%     
==========================================
  Files          25       25              
  Lines        2698     2707       +9     
  Branches      519      522       +3     
==========================================
+ Hits         2319     2328       +9     
  Misses        285      285              
  Partials       94       94              
Impacted Files Coverage Δ
pycardano/nativescript.py 98.03% <96.29%> (+0.18%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@daehan-koreapool
Copy link
Contributor Author

Due to the dynamic nature of NativeScript class which refers to parent from_primitive() method and locking the return type as its subclass, mypy seems to have trouble inferring variable types.

Updates

  • Ensure only list/tuples are accepted for from_primitive()
  • Type ignore statements mypy is having trouble with
  • Ensure mypy understands to_cbor("bytes") returns a bytes value
  • Improve to_dict() return type declaration
  • before and after dataclass attributes is now required fields

@daehan-koreapool daehan-koreapool marked this pull request as ready for review November 13, 2022 05:19
… by individual conditional check

UPDATE. simplify from_dict() method

UPDATE. create a JSON_TAG_TO_INT map
@daehan-koreapool
Copy link
Contributor Author

I was able to avoid all the # type: ignore by breaking up a for-loop with individual super calls depending on the type.

Also, identified that from_dict() can be simplified further reducing code complexity significantly.

@daehan-koreapool daehan-koreapool merged commit 5340be1 into main Nov 13, 2022
@daehan-koreapool daehan-koreapool deleted the enhance-nativescript-typehint branch November 16, 2022 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants