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

undo some unnecessary int type casts #17521

Merged
merged 4 commits into from Feb 8, 2024
Merged

undo some unnecessary int type casts #17521

merged 4 commits into from Feb 8, 2024

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Feb 8, 2024

Purpose:

In recent changes to transition python classes to rust types, rust's python bindings exposed all integers as plain python int, requiring explicit casts in the python code.

Now that rust's python bindings expose integer types as the correct fixed-width integer type, these type casts are unnecessary.

Current Behavior:

We have unnecessary type casts.

New Behavior:

Some unnecessary type casts have been removed.

@arvidn arvidn changed the title undo some redundant int type casts undo some unnecessary int type casts Feb 8, 2024
@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Feb 8, 2024
@arvidn arvidn marked this pull request as ready for review February 8, 2024 17:31
@arvidn arvidn requested a review from a team as a code owner February 8, 2024 17:31
@arvidn arvidn requested a review from altendky February 8, 2024 17:31
altendky
altendky previously approved these changes Feb 8, 2024
chia/full_node/weight_proof.py Outdated Show resolved Hide resolved
chia/full_node/weight_proof.py Outdated Show resolved Hide resolved
chia/wallet/wallet_blockchain.py Outdated Show resolved Hide resolved
@arvidn
Copy link
Contributor Author

arvidn commented Feb 8, 2024

@altendky you're right that these type annotations aren't necessary. I didn't intentionally add them, I just restored this code to the state before I made any changes. I'll remove them though

@arvidn
Copy link
Contributor Author

arvidn commented Feb 8, 2024

I think the missing coverage is reasonable. This is undoing recent changes (that also landed without test coverage)

@arvidn arvidn added the ready_to_merge Submitter and reviewers think this is ready label Feb 8, 2024
Copy link
Contributor

github-actions bot commented Feb 8, 2024

File Coverage Missing Lines
chia/full_node/full_node_api.py 0.0% lines 954, 956
chia/timelord/timelord.py 66.7% lines 282
chia/timelord/timelord_state.py 50.0% lines 90, 93-94
chia/wallet/wallet_blockchain.py 66.7% lines 167
Total Missing Coverage
61 lines 7 lines 88%

@Starttoaster Starttoaster merged commit fe89b5b into main Feb 8, 2024
270 of 271 checks passed
@Starttoaster Starttoaster deleted the rust-int-types branch February 8, 2024 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants