Improve base + blockfrost module maintainability #120

daehan-koreapool · 2022-11-03T01:41:07Z

Background

Optional attributes in GenesisParameters and ProtocolParameters raises various errors.
Updating these dataclasses to be non-optional and frozen/immutable would produce less errors in the future in my opinion.

Goals

Update GenesisParameters and ProtocolParameters to be non-optional and frozen

$ make qa
poetry run flake8 pycardano
poetry run mypy --install-types --non-interactive pycardano
pycardano/backend/base.py:26: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:28: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:30: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:32: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:34: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:36: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:38: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:40: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:42: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:44: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:51: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:53: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:55: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:57: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:59: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:61: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:63: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:65: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:67: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:69: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:71: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:73: error: Incompatible types in assignment (expression has type "None", variable has type "str")
pycardano/backend/base.py:75: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:77: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:79: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:81: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:83: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:85: error: Incompatible types in assignment (expression has type "None", variable has type "float")
pycardano/backend/base.py:87: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:89: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:91: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:93: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:95: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:97: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:99: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:101: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:103: error: Incompatible types in assignment (expression has type "None", variable has type "int")
pycardano/backend/base.py:105: error: Incompatible types in assignment (expression has type "None", variable has type "Dict[str, Dict[str, int]]")
pycardano/backend/blockfrost.py:76: error: Incompatible return value type (got "None", expected "int")
pycardano/backend/blockfrost.py:87: error: Incompatible types in assignment (expression has type "GenesisParameters", variable has type "None")
pycardano/backend/blockfrost.py:88: error: Incompatible return value type (got "None", expected "GenesisParameters")
pycardano/backend/blockfrost.py:94: error: Incompatible types in assignment (expression has type "ProtocolParameters", variable has type "None")
pycardano/backend/blockfrost.py:126: error: Incompatible return value type (got "None", expected "ProtocolParameters")
pycardano/backend/blockfrost.py:141: error: Invalid self argument "Type[NativeScript]" to attribute function "from_dict" with type "Callable[[NativeScript, Dict[Any, Any], bool], Union[ScriptPubkey, ScriptAll, ScriptAny, ScriptNofK, InvalidBefore, InvalidHereAfter]]"
pycardano/backend/blockfrost.py:170: error: Argument 1 to "Value" has incompatible type "Optional[int]"; expected "int"
Found 45 errors in 2 files (checked 8 source files)
make: *** [Makefile:65: qa] Error 1

…nstantiation step

codecov-commenter · 2022-11-03T18:10:11Z

Codecov Report

Merging #120 (4989531) into main (69349fb) will increase coverage by 0.09%.
The diff coverage is 90.62%.

@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
+ Coverage   85.81%   85.91%   +0.09%     
==========================================
  Files          24       25       +1     
  Lines        2664     2690      +26     
  Branches      519      517       -2     
==========================================
+ Hits         2286     2311      +25     
- Misses        284      286       +2     
+ Partials       94       93       -1

Impacted Files	Coverage Δ
pycardano/backend/blockfrost.py	`33.61% <61.53%> (+3.61%)`	⬆️
pycardano/backend/ogmios.py	`55.18% <77.77%> (+1.08%)`	⬆️
pycardano/backend/base.py	`100.00% <100.00%> (ø)`
pycardano/nativescript.py	`97.84% <100.00%> (+1.46%)`	⬆️
pycardano/types.py	`100.00% <100.00%> (ø)`

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

…och value

…ove readability of NativeScript.from_dict() class method

UPDATE. improve NativeScript.from_dict() by pulling out primitive list building step

…meters

daehan-koreapool · 2022-11-04T20:49:30Z

Major Updates

Enforcing GenesisParameters and ProtocolParameters to be frozen dataclasses with non-optional attributes
- Refactor Ogmios context ProtocolParameter instantiation.
- Adding missing ProtocolParameter.min_pool_cost for Blockfrost context
NativeScript
- from_dict() method used to return either List or NativeScript instance depending on top_level boolean field. Removed this ambiguity by pulling out List generating logic into a separate helper method.
- Adding a new testcase to ensure nested native scripts are parsed correctly

Minor Updates

base.py and blockfrost.py are included in mypy analysis

pycardano/types.py

…esented

pycardano/backend/blockfrost.py

pycardano/types.py

cffls

Thanks for the explanation regarding JSON, I just learnt a new thing :) The new naming looks good to me.

pycardano/backend/blockfrost.py

daehan-koreapool · 2022-11-07T22:02:50Z

Hi @cffls
Please merge this one if everything looks good from your perspective.

cffls · 2022-11-07T22:18:53Z

I thought you had permission to merge. Let me double check. Thanks for the reminder.

daehan-koreapool added 5 commits November 3, 2022 11:04

UPDATE. including base.py and blockfrost.py in mypy check

8894624

UPDATE. converting dataclass DTOs to have concrete values at all times

142369d

REFACTOR. provide all required values for ProtocolParameters in the i…

ac95e68

…nstantiation step

ADD. adding ogmios specific integration tests

9386072

ADD. adding a stub extra_entropy value for FixedChainContext

c9a8ba6

daehan-koreapool force-pushed the refactor-base branch from 9da4bdb to c9a8ba6 Compare November 3, 2022 18:05

daehan-koreapool added 9 commits November 3, 2022 14:40

REFACTOR. pulling out JSON type to a dedicated types module

1f52577

ADD. adding type hints for BlockFrostChainContext attributes

d7112ec

UPDATE. explicitly specifying type information of Blockfrost API's ep…

dc27e75

…och value

ADD. adding min_pool_cost to blockfrost ProtocolParameters instantiation

0f35dd1

UPDATE. lovelace_amount variable should be an integer value

a6f5485

ADD. adding a nested Nativescript test case before attempting to impr…

713cad9

…ove readability of NativeScript.from_dict() class method

ADD. adding ogmios parsing integration test

8e4ab25

UPDATE. improve NativeScript.from_dict() by pulling out primitive list building step

UPDATE. renaming script_json serializing method name

782aa40

UPDATE. enforcing immutability for GenesisParameters and ProtocolPara…

7373fd1

…meters

daehan-koreapool marked this pull request as ready for review November 4, 2022 20:49

cffls reviewed Nov 5, 2022

View reviewed changes

pycardano/types.py Outdated Show resolved Hide resolved

daehan-koreapool added 2 commits November 5, 2022 22:04

REFACTOR. renaming JSON type to JsonDict to infer JSON object is repr…

345f62d

…esented

Merge branch 'main' into refactor-base

4989531

daehan-koreapool requested a review from cffls November 6, 2022 05:52

daehan-koreapool commented Nov 6, 2022

View reviewed changes

pycardano/backend/blockfrost.py Show resolved Hide resolved

pycardano/types.py Outdated Show resolved Hide resolved

pycardano/types.py Outdated Show resolved Hide resolved

cffls approved these changes Nov 6, 2022

View reviewed changes

pycardano/backend/blockfrost.py Show resolved Hide resolved

cffls merged commit 7787695 into Python-Cardano:main Nov 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve base + blockfrost module maintainability #120

Improve base + blockfrost module maintainability #120

daehan-koreapool commented Nov 3, 2022

codecov-commenter commented Nov 3, 2022 •

edited

Loading

daehan-koreapool commented Nov 4, 2022

cffls left a comment

daehan-koreapool commented Nov 7, 2022

cffls commented Nov 7, 2022

Improve base + blockfrost module maintainability #120

Improve base + blockfrost module maintainability #120

Conversation

daehan-koreapool commented Nov 3, 2022

Background

Goals

codecov-commenter commented Nov 3, 2022 • edited Loading

Codecov Report

daehan-koreapool commented Nov 4, 2022

Major Updates

Minor Updates

cffls left a comment

Choose a reason for hiding this comment

daehan-koreapool commented Nov 7, 2022

cffls commented Nov 7, 2022

codecov-commenter commented Nov 3, 2022 •

edited

Loading