Skip to content

fix: preserve hand-written Python package initializers - #2590

Merged
Rana Singh (ranadeepsingh) merged 6 commits into
microsoft:masterfrom
ranadeepsingh:copilot/ancient-2286-init-codegen
Aug 7, 2026
Merged

fix: preserve hand-written Python package initializers#2590
Rana Singh (ranadeepsingh) merged 6 commits into
microsoft:masterfrom
ranadeepsingh:copilot/ancient-2286-init-codegen

Conversation

@ranadeepsingh

@ranadeepsingh Rana Singh (ranadeepsingh) commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Related PRs

Problem

PyCodegen copies hand-written Python overrides and then rewrites nested __init__.py files, discarding required manual content. It also deletes every namespace-root initializer and the prepend approach proposed in #2286 would duplicate generated headers when init generation is repeated.

Before the fix, targeted regression tests reproduced three failures: nested manual UTF-8 content was missing, a non-empty manual namespace root was deleted, and OpenAI manual content was lost around the #2560 hook.

Changes

  • Read manual initializer content from the source override tree, never from generated output.
  • Emit generated imports in deterministic filename order using explicit UTF-8.
  • Compose generated package metadata/imports, initFileExtra, then manual content.
  • Keep empty/absent namespace roots as namespace packages while retaining non-empty manual roots.
  • Keep the cognitive compatibility initializer entirely hand-written.
  • Add focused tests for roots, nested packages, UTF-8, manual presence/absence, OpenAI extras, and repeat-run idempotency.

No files under target/ or other generated output are committed.

Validation

  • core/testOnly com.microsoft.azure.synapse.ml.codegen.PyCodegenSuite: 11 passed.
  • core/Test/compile and cognitive/Test/compile: passed.
  • Canonical core/codegen, cognitive/codegen, and deepLearning/codegen: passed.
  • core/scalastyle and core/Test/scalastyle: passed with zero findings.
  • black==22.3.0 --check --extend-exclude docs/ .: 179 files unchanged.
  • Strict UTF-8 generated-output assertions passed for namespace roots, manual suffixes, OpenAI hook/import ordering, and the cognitive compatibility package.
  • Repeated fixed-version core/codegen was byte-idempotent across all 30 generated __init__.py files.

Review

The repository code-review checklist was run before commit; no concrete findings remained.

CI iteration

The first Azure Pipelines run exposed a stale source override at core/src/main/python/synapse/ml/automl/__init__.py: it imported _FindBestModel, but that generated module does not exist. The file had previously been harmless only because PyCodegen overwrote it. The override is now removed so deterministic codegen supplies the valid FindBestModel import. Local targeted tests and canonical core codegen passed after the fix.

Packaging review follow-up

Independent review found that find_namespace_packages(include=["synapse.ml.*"]) omitted a preserved synapse/ml/__init__.py from wheels. Component and aggregate setup templates now include both the exact synapse.ml package and synapse.ml.*. Regression tests build and inspect real fixture wheels for both templates, including UTF-8 root initializer content.

Codegen shard follow-up

Enabling com.microsoft.azure.synapse.ml.codegen.** exposed a dormant WrappableTests assertion that still expected TestRegressorModel to be nested under WrappableTests, even though #2195 moved the fixture to a package-level source file. The assertion now verifies the actual reflected top-level name. The correction is intentionally limited to that one line so the equivalent Spark 4.1 change replays cleanly. The combined WrappableTests and PyCodegenSuite run passes 12/12 tests.

The actual PR patch now applies cleanly onto spark4.1, including the codegen production and test sources.

Copilot AI lite review requested due to automatic review settings August 1, 2026 12:03
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Hey Rana Singh (@ranadeepsingh) 👋!
Thank you so much for contributing to our repository 🙌.
Someone from SynapseML Team will be reviewing this pull request soon.

We use semantic commit messages to streamline the release process.
Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix.
This helps us to create release messages and credit you for your hard work!

Examples of commit messages with semantic prefixes:

  • fix: Fix LightGBM crashes with empty partitions
  • feat: Make HTTP on Spark back-offs configurable
  • docs: Update Spark Serving usage
  • build: Add codecov support
  • perf: improve LightGBM memory usage
  • refactor: make python code generation rely on classes
  • style: Remove nulls from CNTKModel
  • test: Add test coverage for CNTKModel

To test your commit locally, please follow our guild on building from source.
Check out the developer guide for additional guidance on testing your change.

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates SynapseML’s Python code generation (PyCodegen) to preserve hand-written __init__.py content (including UTF-8 text) while still emitting deterministic generated imports and required OpenAI compatibility glue. It also adds targeted regression tests to validate manual-content retention, namespace-root behavior, ordering, and idempotency across repeated runs.

Changes:

  • Refactors PyCodegen.makeInitFiles to read manual initializer content from the source override tree (src/main/python) and compose it after generated metadata/imports (and any initFileExtra).
  • Makes generated import emission deterministic (sorted) and explicitly UTF-8 encoded for init-file writes/reads.
  • Adds PyCodegenSuite covering UTF-8 preservation, namespace-root deletion/retention rules, OpenAI hook ordering, cognitive manual-only behavior, and repeat-run byte idempotency.
Show a summary per file
File Description
core/src/main/scala/com/microsoft/azure/synapse/ml/codegen/PyCodegen.scala Refactors init-file generation to preserve manual content from the override tree, enforce deterministic ordering, and use explicit UTF-8.
core/src/test/scala/com/microsoft/azure/synapse/ml/codegen/PyCodegenSuite.scala Adds focused regression tests for manual __init__.py preservation, namespace-root handling, UTF-8, OpenAI hook behavior, and idempotency.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.65%. Comparing base (d3ef6e3) to head (0d1f4f2).

Files with missing lines Patch % Lines
...ft/azure/synapse/ml/codegen/PythonInitMerger.scala 87.09% 16 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2590      +/-   ##
==========================================
- Coverage   86.07%   82.65%   -3.42%     
==========================================
  Files         331      332       +1     
  Lines       17654    17780     +126     
  Branches     1646     1607      -39     
==========================================
- Hits        15195    14696     -499     
- Misses       2459     3084     +625     

☔ View full report in Codecov by Harness.
📢 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.

Rana Singh (ranadeepsingh) added a commit to ranadeepsingh/SynapseML that referenced this pull request Aug 1, 2026
## Summary
Remove the obsolete automl initializer override that imports the nonexistent _FindBestModel module. With manual initializer preservation enabled, that previously hidden dead code broke AutoML imports in website and Databricks checks.

## Prompting Intent
Iterate on every PR check until green without editing generated output. Diagnose the failures caused by preserving current source overrides and fix the source-level defect exposed by the codegen change.

## Linked Sources
- Current PR: microsoft#2590
- Original init preservation proposal: microsoft#2286
- Failing Azure Pipelines build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229204431
- Prior stale-file analysis: microsoft@0b07808

## Rationale
Delete the override instead of replacing it with another manual import. PyCodegen already generates the valid FindBestModel wildcard import from FindBestModel.py, while _FindBestModel.py has never existed on this branch. Removing dead source keeps generated imports authoritative and avoids preserving a broken compatibility symbol.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Rana Singh (ranadeepsingh) added a commit to ranadeepsingh/SynapseML that referenced this pull request Aug 1, 2026
## Summary
Include both synapse.ml and synapse.ml.* in component and aggregate setuptools namespace discovery so a preserved non-empty synapse/ml/__init__.py is shipped in wheels. Add component and aggregate wheel-content regression fixtures.

## Prompting Intent
Address the independent PR review finding that init preservation was incomplete at the packaging boundary, validate actual wheel contents, push the existing PR, and rerun all checks to green.

## Linked Sources
- Current PR: microsoft#2590
- Original init preservation proposal: microsoft#2286
- OpenAI codegen compatibility change: microsoft#2560

## Rationale
Setuptools include patterns do not make synapse.ml.* match the synapse.ml package itself. Listing the exact namespace root alongside the descendant wildcard preserves namespace behavior when no root initializer exists while packaging the root whenever codegen retains a non-empty manual initializer. Building and inspecting real fixture wheels guards both setup templates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

Ilya Matiach (@imatiach-msft) Re-review requested after addressing the namespace-root wheel packaging finding.

Commit 484c24f5ea now includes both synapse.ml and synapse.ml.* in component and aggregate discovery. The new regression coverage builds and inspects both wheel variants and verifies the preserved UTF-8 synapse/ml/__init__.py content.

Local validation: 7/7 targeted PyCodegen tests passed, component/test scalastyle passed with zero findings, Black passed, and canonical core codegen emitted the corrected discovery expression.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Comment thread core/src/main/python/synapse/ml/automl/__init__.py
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

## Summary
Generate deterministic UTF-8 Python package import headers while preserving hand-written __init__.py source overrides, namespace roots, and the cognitive compatibility package. Add regression coverage for nested packages, manual and absent initializers, OpenAI extras, and repeat-run idempotency.

## Prompting Intent
Recreate the intent of stale PR microsoft#2286 on current master without editing generated outputs, account for the OpenAI init hook added by microsoft#2560, and validate the change through TDD, compilation, codegen, style, and generated-output assertions.

## Linked Sources
- Original fix proposal: microsoft#2286
- OpenAI codegen change: microsoft#2560

## Rationale
Read manual initializer content from the source override tree rather than prepending to generated output. This keeps source authoritative and prevents duplicate generated blocks on repeated codegen. Generated imports remain sorted, the microsoft#2560 hook stays between generated imports and manual content, non-empty manual namespace roots are retained, and empty roots remain namespace packages.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary
Remove the obsolete automl initializer override that imports the nonexistent _FindBestModel module. With manual initializer preservation enabled, that previously hidden dead code broke AutoML imports in website and Databricks checks.

## Prompting Intent
Iterate on every PR check until green without editing generated output. Diagnose the failures caused by preserving current source overrides and fix the source-level defect exposed by the codegen change.

## Linked Sources
- Current PR: microsoft#2590
- Original init preservation proposal: microsoft#2286
- Failing Azure Pipelines build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=229204431
- Prior stale-file analysis: microsoft@0b07808

## Rationale
Delete the override instead of replacing it with another manual import. PyCodegen already generates the valid FindBestModel wildcard import from FindBestModel.py, while _FindBestModel.py has never existed on this branch. Removing dead source keeps generated imports authoritative and avoids preserving a broken compatibility symbol.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary
Include both synapse.ml and synapse.ml.* in component and aggregate setuptools namespace discovery so a preserved non-empty synapse/ml/__init__.py is shipped in wheels. Add component and aggregate wheel-content regression fixtures.

## Prompting Intent
Address the independent PR review finding that init preservation was incomplete at the packaging boundary, validate actual wheel contents, push the existing PR, and rerun all checks to green.

## Linked Sources
- Current PR: microsoft#2590
- Original init preservation proposal: microsoft#2286
- OpenAI codegen compatibility change: microsoft#2560

## Rationale
Setuptools include patterns do not make synapse.ml.* match the synapse.ml package itself. Listing the exact namespace root alongside the descendant wildcard preserves namespace behavior when no root initializer exists while packaging the root whenever codegen retains a non-empty manual initializer. Building and inspecting real fixture wheels guards both setup templates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary
Preserve legal hand-written Python initializer prologues around deterministic generated imports, run codegen regressions in PR CI, and make release-branch replay ignore deletions that are already absent.

## Prompting Intent
Rebase PR microsoft#2590 onto current master, fix the reviewed Python prologue and test-routing defects without restoring obsolete imports, keep code generation deterministic and fast, and ensure the Spark 4.1 compatibility replay remains meaningful.

## Linked Sources
- Pull request: microsoft#2590
- Prior initializer preservation work: microsoft#2286
- OpenAI compatibility hook: microsoft#2560

## Rationale
A small deterministic Scala scanner preserves encoding declarations, module docstrings, and future imports without adding a parser dependency, while generated exports remain authoritative and manual overrides stay last. The release replay filters only pure deletions absent from both trees, avoiding branch-specific hacks without hiding applicable changes. Adding the existing codegen suite to the core shard prevents future regressions from compiling without executing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 81d39bfc-927c-418a-90a8-e0f2cd8fc128
Copilot AI review requested due to automatic review settings August 7, 2026 07:55
@ranadeepsingh
Rana Singh (ranadeepsingh) force-pushed the copilot/ancient-2286-init-codegen branch from 9eadc7e to e39b01e Compare August 7, 2026 07:55
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 6/7 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

## Summary
Update the previously dormant Wrappable companion-model test to expect the actual top-level TestRegressorModel name and remove imports left behind when the fixture moved to its own source file.

## Prompting Intent
Prepare PR microsoft#2590 for merge by diagnosing and fixing its Azure core-unit failure after codegen tests were added to the normal PR shard. Keep the correction narrow and preserve the PR's goal of making all code generation behavior continuously validated.

## Linked Sources
- Pull request: microsoft#2590
- Failed Azure build: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=230147126
- Original companion-model fix: microsoft#2195

## Rationale
The fixture has been a package-level class since microsoft#2195, so Scala reflection correctly returns com.microsoft.azure.synapse.ml.codegen.TestRegressorModel. The obsolete nested-class expectation went unnoticed because the codegen package was not part of the core CI shard. Correcting the assertion is preferable to excluding the suite or changing production reflection logic that already returns the right public class name.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 81d39bfc-927c-418a-90a8-e0f2cd8fc128
Copilot AI review requested due to automatic review settings August 7, 2026 09:21
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Suppressed comments (2)

core/src/test/scala/com/microsoft/azure/synapse/ml/codegen/PyCodegenSuite.scala:85

  • buildWheel hard-depends on an external Python environment having both setuptools and wheel available. If either is missing (or python3 isn’t present), this Scala unit test will fail with a packaging error instead of being skipped with a clear reason, which can make CI and local runs flaky.

Consider pre-checking for required Python packaging deps and using assume(...) to skip the wheel-building assertions when unavailable.

  private def buildWheel(sourceDir: File, wheelDir: File): File = {
    wheelDir.mkdirs()
    val process = new ProcessBuilder(
      pythonExecutable, "setup.py", "bdist_wheel", "--universal", "-d", wheelDir.getAbsolutePath)
      .directory(sourceDir)
      .redirectErrorStream(true)
      .start()

core/src/main/scala/com/microsoft/azure/synapse/ml/codegen/PyCodegen.scala:159

  • generateInitFiles is intended to make init generation deterministic and UTF-8 safe, but makeInitFiles still writes via FileUtilities.writeFile, which uses stuff.toString.getBytes() (platform default charset). On systems where the default charset isn’t UTF-8 (notably some Windows setups), this can produce non-UTF-8 __init__.py bytes for packages that don’t have a manual override (and therefore never get rewritten by PythonInitMerger).

To make encoding guarantees consistent, consider writing generated init files with StandardCharsets.UTF_8 (either by updating writeFile to accept a charset or by using Files.write(..., UTF_8) in makeInitFiles).

  private[codegen] def generateInitFiles(conf: CodegenConfig): Unit = {
    makeInitFiles(conf)
    PythonInitMerger.preserve(conf)
  }
  • Files reviewed: 7/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

## Summary
Limit the dormant Wrappable test correction to the stale expected class name, retaining the surrounding release-branch source shape so the already-equivalent Spark 4.1 change merges without conflict.

## Prompting Intent
Prepare PR microsoft#2590 for merge after Azure confirmed that its product and codegen changes replayed onto Spark 4.1 but the broader test cleanup conflicted with an equivalent release-branch assertion fix. Preserve the corrected master test while keeping the compatibility patch minimal.

## Linked Sources
- Pull request: microsoft#2590
- Failed release replay: https://msdata.visualstudio.com/b9b2accc-2d1c-45b3-9d24-0eb5d78cc47f/_build/results?buildId=230156549
- Original companion-model fix: microsoft#2195

## Rationale
Spark 4.1 already expects the top-level TestRegressorModel name. Reverting unrelated import and formatting cleanup leaves a one-line semantic correction whose three-way merge recognizes that identical release change, while continuing to make the newly enabled master test pass. This avoids adding release-specific exclusions or weakening codegen CI coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 81d39bfc-927c-418a-90a8-e0f2cd8fc128
Copilot AI review requested due to automatic review settings August 7, 2026 09:46
@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

Suppressed comments (1)

core/src/test/scala/com/microsoft/azure/synapse/ml/codegen/PyCodegenSuite.scala:85

  • buildWheel assumes the bdist_wheel setuptools command is available, but that command only exists when the Python wheel package is installed. In environments where wheel is missing, this fails with a non-obvious "invalid command 'bdist_wheel'" error, which can make CI/local runs flaky and harder to diagnose.

Consider adding an explicit preflight check that wheel is importable (and fail with a clear message) before invoking setup.py bdist_wheel.

  private def buildWheel(sourceDir: File, wheelDir: File): File = {
    wheelDir.mkdirs()
    val process = new ProcessBuilder(
      pythonExecutable, "setup.py", "bdist_wheel", "--universal", "-d", wheelDir.getAbsolutePath)
      .directory(sourceDir)
      .redirectErrorStream(true)
  • Files reviewed: 7/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ranadeepsingh

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@ranadeepsingh
Rana Singh (ranadeepsingh) enabled auto-merge (squash) August 7, 2026 22:26
@ranadeepsingh Rana Singh (ranadeepsingh) changed the title fix: preserve hand-written Python package initializers wip: preserve hand-written Python package initializers Aug 7, 2026
@ranadeepsingh Rana Singh (ranadeepsingh) changed the title wip: preserve hand-written Python package initializers fix: preserve hand-written Python package initializers Aug 7, 2026
@ranadeepsingh
Rana Singh (ranadeepsingh) merged commit 459f01e into microsoft:master Aug 7, 2026
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants