Skip to content

refactor(pyamber): flatten over-nested util packages#4952

Merged
Yicong-Huang merged 3 commits intoapache:mainfrom
Yicong-Huang:chore/flatten-pyamber-util-packages
May 6, 2026
Merged

refactor(pyamber): flatten over-nested util packages#4952
Yicong-Huang merged 3 commits intoapache:mainfrom
Yicong-Huang:chore/flatten-pyamber-util-packages

Conversation

@Yicong-Huang
Copy link
Copy Markdown
Contributor

@Yicong-Huang Yicong-Huang commented May 5, 2026

What changes were proposed in this PR?

Flatten core/util/<X>/<X>.py single-file directories — the "one class per directory" pattern that was transplanted from Java/Scala — into flat modules under core/util/. Also extract the implementations that were living directly inside __init__.py (expression_evaluator, virtual_identity) into named modules.

Any related issues, documentation, discussions?

Closes #4951.

How was this PR tested?

pytest core/util (60 passed, 1 xfailed) and pytest core/runnables (34 passed) green locally.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 4.7 (Claude Code)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.58%. Comparing base (933f7fe) to head (e814e83).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4952      +/-   ##
============================================
- Coverage     42.58%   42.58%   -0.01%     
+ Complexity     2184     2183       -1     
============================================
  Files          1031     1005      -26     
  Lines         38110    37436     -674     
  Branches       4004     3918      -86     
============================================
- Hits          16231    15941     -290     
+ Misses        20863    20527     -336     
+ Partials       1016      968      -48     
Flag Coverage Δ
python 88.90% <100.00%> (+0.88%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Yicong-Huang Yicong-Huang requested a review from aglinxinyuan May 5, 2026 18:52
@Yicong-Huang
Copy link
Copy Markdown
Contributor Author

I need to add more tests. will do it later.

Several `core/util/<X>/<X>.py` directories existed only to host a single
file — a Java/Scala "one class per directory" convention transplanted
into Python where it adds noise without value. Two related anti-patterns
existed alongside it:

- `core/util/expression_evaluator/__init__.py` was 211 lines of evaluator
  implementation (the package had no other modules).
- `core/util/virtual_identity/__init__.py` was 99 lines of helper functions.

`__init__.py` should re-export, not host the implementation.

Changes:

- Flatten single-file directories to flat modules:
  `runnable/runnable.py`, `thread/atomic.py`, `protocol/base_protocols.py`
  → `runnable.py`, `atomic.py`, `base_protocols.py`.
- Delete `core/util/operator/` — `__init__.py` was a 16-line license header
  with no exports and no importers.
- Move `expression_evaluator/__init__.py` and `virtual_identity/__init__.py`
  implementations into flat single-file modules (the dirs only ever held
  the impl + the test file).
- Move tests up alongside the new flat modules.
- Replace the hardcoded `core.util.expression_evaluator.test_expression_evaluator`
  module path inside `test_evaluate_object_expression` with `A.__module__`
  so the test tracks whatever import path it resolves to (it would have
  broken on this rename otherwise).
- Inside `core/util/stoppable/stoppable_queue_blocking_thread.py`, switch
  the intra-package `from core.util.stoppable import Stoppable` back to
  the relative `from .stoppable import Stoppable` to avoid hitting the
  parent package's __init__ mid-initialization.

Closes apache#4951
… drain

The util-flatten rename touched `core/python_worker.py`'s import lines,
and codecov flagged them because the file had no test coverage at all.
Add a focused unit test that stubs `NetworkReceiver`, `NetworkSender`,
`MainLoop`, and `Heartbeat`, then exercises:

- construction wires the receiver's proxy port into the sender, and
  the receiver's shutdown callback is bound to `worker.stop`
- `stop()` cascades to main loop, sender, and heartbeat
- `run()` sets the heartbeat stop event after main loop returns

Brings `core/python_worker.py` to 100% line coverage.
@Yicong-Huang Yicong-Huang force-pushed the chore/flatten-pyamber-util-packages branch from 25eda94 to ded8661 Compare May 6, 2026 07:02
@Yicong-Huang Yicong-Huang enabled auto-merge (squash) May 6, 2026 07:06
@Yicong-Huang Yicong-Huang merged commit 2d6a759 into apache:main May 6, 2026
16 checks passed
@Yicong-Huang Yicong-Huang deleted the chore/flatten-pyamber-util-packages branch May 6, 2026 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flatten over-nested single-file packages and __init__.py-only implementations under amber/src/main/python/core/util/

3 participants