refactor: remove deprecated in_tinybars parameter
#1188
Conversation
…d tests Signed-off-by: MonaaEid <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1188 +/- ##
==========================================
+ Coverage 91.26% 91.28% +0.02%
==========================================
Files 139 139
Lines 8447 8447
==========================================
+ Hits 7709 7711 +2
+ Misses 738 736 -2 🚀 New features to boost your workflow:
|
…d tests Signed-off-by: MonaaEid <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoved the deprecated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/hiero_sdk_python/tokens/custom_fixed_fee.py (1)
90-113: Well-implemented deprecation with proper warning.The deprecation is correctly implemented:
- Uses
DeprecationWarningwith appropriatestacklevel=2- Provides clear migration guidance in both docstring and warning message
- Maintains backward compatibility while signaling future removal
One minor improvement: the
.. deprecated::Sphinx directive on line 93 is missing a version number. Consider adding the version where this was deprecated for documentation clarity.🔎 Add version to deprecation directive
- .. deprecated:: + .. deprecated:: 0.2.0 Use :meth:`set_hbar_amount` with :meth:`Hbar.from_tinybars` instead. For example: ``set_hbar_amount(Hbar.from_tinybars(amount))``src/hiero_sdk_python/hbar.py (2)
35-42: Stale docstring reference to removed parameter.The docstring still mentions
in_tinybars (deprecated)on line 41, but this parameter has been removed. The docstring should be updated to reflect the current API.🔎 Update docstring
""" Create an Hbar instance with the given amount designated either in hbars or tinybars. Args: amount: The numeric amount of hbar or tinybar. unit: Unit of the provided amount. - in_tinybars (deprecated): If True, treat the amount as tinybars directly. """
9-10: Unused import.The
warningsmodule is imported but no longer used after removing thein_tinybarsdeprecation logic. This should be cleaned up.🔎 Remove unused import
import re -import warnings from decimal import Decimaltests/unit/hbar_test.py (1)
24-34: Tests correctly updated to use newunitparameter.The tests properly verify the new API using
unit=HbarUnit.TINYBARinstead of the removedin_tinybarsparameter.Note:
test_constructor_in_tinybars(lines 24-28) is now functionally identical to the first case intest_constructor_with_unit(lines 32-34). Consider consolidating these tests or renamingtest_constructor_in_tinybarsto better reflect that it's testing theunit=HbarUnit.TINYBARpath.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)src/hiero_sdk_python/hbar.py(3 hunks)src/hiero_sdk_python/tokens/custom_fixed_fee.py(3 hunks)tests/unit/custom_fee_test.py(3 hunks)tests/unit/hbar_test.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/unit/hbar_test.py (2)
src/hiero_sdk_python/hbar.py (1)
Hbar(18-166)src/hiero_sdk_python/hbar_unit.py (1)
HbarUnit(3-47)
tests/unit/custom_fee_test.py (1)
src/hiero_sdk_python/tokens/custom_fixed_fee.py (2)
CustomFixedFee(22-300)set_amount_in_tinybars(90-113)
src/hiero_sdk_python/hbar.py (1)
src/hiero_sdk_python/hbar_unit.py (1)
HbarUnit(3-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Agent
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.12)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (2)
src/hiero_sdk_python/hbar.py (1)
108-110: LGTM!The
from_tinybarsmethod is correctly updated to useunit=HbarUnit.TINYBARinstead of the removedin_tinybarsparameter.tests/unit/hbar_test.py (1)
60-63: LGTM!The fractional tinybar test correctly uses the new
unit=HbarUnit.TINYBARparameter.
There was a problem hiding this comment.
Pull request overview
This pull request removes the deprecated in_tinybars parameter from the Hbar class constructor and updates all code and tests to use the unit parameter with HbarUnit.TINYBAR instead. Additionally, it deprecates the set_amount_in_tinybars() method in CustomFixedFee with an appropriate warning.
Key changes:
- Removed the deprecated
in_tinybarsboolean parameter and related logic from theHbar.__init__()method - Updated
Hbar.from_tinybars()to useunit=HbarUnit.TINYBARinstead of the removed parameter - Added deprecation warning to
CustomFixedFee.set_amount_in_tinybars()method recommending the use ofset_hbar_amount(Hbar.from_tinybars(amount))instead
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hiero_sdk_python/hbar.py | Removed in_tinybars parameter from constructor and updated from_tinybars() to use unit parameter |
| tests/unit/hbar_test.py | Updated tests to replace in_tinybars=True with unit=HbarUnit.TINYBAR |
| src/hiero_sdk_python/tokens/custom_fixed_fee.py | Added deprecation warning to set_amount_in_tinybars() method |
| tests/unit/custom_fee_test.py | Added test to verify deprecation warning is properly raised |
| CHANGELOG.md | Documented the removal of deprecated parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d tests Signed-off-by: MonaaEid <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/hiero_sdk_python/hbar.py (1)
30-34: Fix PEP 8 style: missing spaces around=in type-annotated default.PEP 8 recommends
unit: HbarUnit = HbarUnit.HBAR(with spaces) when the parameter has a type annotation.🔎 Fix spacing
def __init__( self, amount: Union[int, float, Decimal], - unit: HbarUnit=HbarUnit.HBAR + unit: HbarUnit = HbarUnit.HBAR ) -> None:tests/unit/hbar_test.py (1)
30-31: Update the misleading docstring.The docstring says "Test creation directly in tinybars" but this test actually validates construction with multiple units (TINYBAR, MICROBAR, MILLIBAR, HBAR, KILOBAR, MEGABAR, GIGABAR).
🔎 Suggested fix
def test_constructor_with_unit(): - """Test creation directly in tinybars.""" + """Test creation with various HbarUnit values.""" hbar1 = Hbar(50, unit=HbarUnit.TINYBAR)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/hiero_sdk_python/hbar.py(3 hunks)tests/unit/custom_fee_test.py(3 hunks)tests/unit/hbar_test.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/hiero_sdk_python/hbar.py (1)
src/hiero_sdk_python/hbar_unit.py (1)
HbarUnit(3-47)
tests/unit/custom_fee_test.py (1)
src/hiero_sdk_python/tokens/custom_fixed_fee.py (2)
CustomFixedFee(22-300)set_amount_in_tinybars(90-113)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (4)
tests/unit/custom_fee_test.py (2)
3-3: LGTM!The
warningsimport is necessary for the new deprecation test and is appropriately placed.
259-274: LGTM!The test correctly verifies:
- A
DeprecationWarningis raised with the expected message- The method still functions correctly after deprecation (amount=100, denominating_token_id=None)
The test follows best practices for deprecation testing.
tests/unit/hbar_test.py (1)
60-63: LGTM!The test correctly validates that fractional tinybar values raise a
ValueErrorwhen usingunit=HbarUnit.TINYBAR.src/hiero_sdk_python/hbar.py (1)
96-109: LGTM!The method correctly migrates from the deprecated
in_tinybars=Trueparameter tounit=HbarUnit.TINYBAR, maintaining the same functionality while aligning with the new API.
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Hi @MonaaEid
This is fantastic
The hbarunit functionality was added in 0.1.9, with the deprecation message
We have now released 0.1.10 and hopefully soon can release 0.2.0, in which case it will be appropriate to merge in this on this upcoming release
Could you please emphasise it is a breaking change in the changelog :)
…d tests Signed-off-by: MontyPokemon <[email protected]>
exploreriii
left a comment
exploreriii
left a comment
There was a problem hiding this comment.
Hi @MonaaEid
Nicely done and thanks for picking up instances of the other methods that need to be deprecated :)
Much appreciated! |
Signed-off-by: MonaaEid <[email protected]> Signed-off-by: MontyPokemon <[email protected]> Signed-off-by: prishajaiswal75 <[email protected]>
2 participants
Description:
This pull request removes the deprecated
in_tinybarsparameter from theHbarclass and updates related code and tests to use theunitparameter instead. It also deprecates theset_amount_in_tinybarsmethod in favor of a more explicit approach, and updates documentation and warnings accordingly.Deprecation and Removal of
in_tinybars:in_tinybarsparameter from theHbarclass constructor and all related logic insrc/hiero_sdk_python/hbar.py. All code should now use theunitparameter to specify tinybars.from_tinybarsclass method inHbarto use theunitparameter instead ofin_tinybars.Test Updates:
tests/unit/hbar_test.pyto remove usage ofin_tinybarsand switch to theunitparameter.Deprecation of Tinybar Setter Method:
set_amount_in_tinybarsmethod inCustomFixedFee, adding a warning and updating the docstring to recommend usingset_hbar_amount(Hbar.from_tinybars(amount))instead.Related issue(s):
Fixes #1167
Checklist
in_tinybarsparameter from theHbarclass constructor.