FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: use str instead of float to preserve precision by 1yam · Pull Request #151 · aleph-im/aleph-sdk-python · GitHub

/ aleph-sdk-python Public

fix: use str instead of float to preserve precision - #151

Draft
1yam wants to merge 3 commits into
mainfrom
1yam-fix-price-response
ConversationCommits (3)ChecksFiles changed
Draft

fix: use str instead of float to preserve precision #151
1yam wants to merge 3 commits into
mainfrom
1yam-fix-price-response

Conversation

1yam commented Aug 19, 2024

Copy link
Copy Markdown
Member

This PR addresses an issue where using float to represent certain numerical values resulted in loss of precision due to rounding. By switching to str, we can maintain the exact value without rounding errors.

1yam requested a review from nesitor August 19, 2024 15:06

github-actions Bot commented Aug 19, 2024

Copy link
Copy Markdown

Summary:
The PR involves a small change where the required_tokens type from float to str is updated in the PriceResponse class. This change is unlikely to introduce bugs and is primarily focused on improving data consistency and type safety.

Highlight:

-    required_tokens: float
+    required_tokens: str

This change is straightforward and does not affect the overall functionality of the codebase, making it suitable for a routine review.

github-actions Bot added the BLUE This PR is simple and straightforward. label Aug 19, 2024

1yam commented Aug 19, 2024

Copy link
Copy Markdown
Member Author

Unit test should work need PR merge

hoh marked this pull request as draft August 19, 2024 15:10

hoh commented Aug 19, 2024

Copy link
Copy Markdown
Member

Converted to Draft, this should use the Decimal type in Python and ensuring that the upstream API returns the expected type.

hoh reviewed Aug 20, 2024
View reviewed changes
Comment thread src/aleph/sdk/query/responses.py Outdated
1yam force-pushed the 1yam-fix-price-response branch from f0f6a94 to e9d98f8 Compare August 21, 2024 14:19

Psycojoker commented Aug 27, 2024

Copy link
Copy Markdown
Contributor

Apparently the other PR has been merged, should we resume this PR?

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

hoh hoh left review comments
nesitor Awaiting requested review from nesitor

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

BLUE This PR is simple and straightforward.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL