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

[SPARK-46559][MLLIB] Wrap the export in the package name with backticks #44555

Closed
wants to merge 1 commit into from

Conversation

LuciferYang
Copy link
Contributor

What changes were proposed in this pull request?

This pr wrap the export in the package name with backticks due to export will become keywords in Scala 3.

Why are the changes needed?

export will become keywords in Scala 3, Scala 2.13 compiler will check for relevant cases in the code, but it does not check for cases in the package name. However, if we write a Scala file as follows and compile it with Scala 3,

package org.apache.spark.mllib.pmml.export

private class ExportABC() {}

it will throw an error:

bin/scalac test.scala -explain                                                
-- [E040] Syntax Error: test.scala:1:36 ----------------------------------------
1 |package org.apache.spark.mllib.pmml.export
  |                                    ^^^^^^
  |                                an identifier expected, but 'export' found
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  |
  | If you want to use 'export' as identifier, you may put it in backticks: `export`.
   -----------------------------------------------------------------------------
1 error found

We can workaround by wrapping export with backticks.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

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

No

@github-actions github-actions bot added the MLLIB label Jan 2, 2024
@LuciferYang
Copy link
Contributor Author

cc @zhengruifeng This is a slightly ahead-of-time PR. I found that the modification did not break the mima check, so I fixed this issue on the fly. We can also wait until the Scala 2.13 compiler starts to throw errors or starts to support Scala 3 before fixing this issue.

@zhengruifeng
Copy link
Contributor

also cc @WeichenXu123

@LuciferYang
Copy link
Contributor Author

Merged into master. Thanks @zhengruifeng @HyukjinKwon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants