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

more mypy #125

Merged
merged 61 commits into from
May 7, 2024
Merged

more mypy #125

merged 61 commits into from
May 7, 2024

Conversation

altendky
Copy link
Contributor

@altendky altendky commented Aug 18, 2022

catch up of #107

Broad Concerns:

  • various code results in different exceptions being raised in unexpected cases. for example, an AssertionError if an object is None where previously a TypeError would have been raised where we take the len() of the object.
  • hinting around internal complex stack-related activities that... i should probably mostly accept failure on and ignore

Draft For:

(cherry picked from commit 9741f9c)
(cherry picked from commit 65b0fb8)
(cherry picked from commit f2a0be4)
@altendky altendky mentioned this pull request Nov 20, 2023
clvm/CLVMObject.py Outdated Show resolved Hide resolved
Copy link

coveralls-official bot commented Dec 7, 2023

Pull Request Test Coverage Report for Build 8424848351

Details

  • 237 of 253 (93.68%) changed or added relevant lines in 11 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-1.1%) to 93.997%

Changes Missing Coverage Covered Lines Changed/Added Lines %
clvm/CLVMObject.py 15 16 93.75%
clvm/EvalError.py 4 5 80.0%
clvm/as_python.py 38 39 97.44%
clvm/op_utils.py 6 7 85.71%
clvm/run_program.py 18 19 94.74%
clvm/serialize.py 29 30 96.67%
clvm/SExp.py 34 36 94.44%
clvm/more_ops.py 49 51 96.08%
clvm/operators.py 29 35 82.86%
Files with Coverage Reduction New Missed Lines %
clvm/operators.py 1 65.47%
Totals Coverage Status
Change from base Build 8162235751: -1.1%
Covered Lines: 994
Relevant Lines: 1044

💛 - Coveralls

clvm/SExp.py Outdated Show resolved Hide resolved
clvm/SExp.py Outdated Show resolved Hide resolved
clvm/SExp.py Show resolved Hide resolved
Copy link

@Quexington Quexington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In chia_rs .as_atom() was changed to raise when .atom was None. If we make that same change here, it will simplify a lot of areas it seems.

clvm/SExp.py Show resolved Hide resolved
clvm/SExp.py Show resolved Hide resolved
clvm/SExp.py Show resolved Hide resolved
clvm/more_ops.py Show resolved Hide resolved
Copy link
Contributor Author

@altendky altendky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking.

clvm/SExp.py Show resolved Hide resolved
clvm/SExp.py Show resolved Hide resolved
clvm/SExp.py Show resolved Hide resolved
clvm/more_ops.py Show resolved Hide resolved
@altendky altendky marked this pull request as ready for review May 7, 2024 14:05
@altendky altendky requested a review from Quexington May 7, 2024 14:06
@altendky altendky merged commit 90a7495 into main May 7, 2024
21 checks passed
@altendky altendky deleted the mypie branch May 7, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants