Skip to content

Conversation

jsleight
Copy link
Contributor

The enums for binary operation (and others) are implemented as nested case objects, which means they have class paths like the following:

$ ls mleap-core/target/scala-2.12/classes/ml/combust/mleap/core/feature/ | grep BinaryOperation
BinaryOperation$.class
BinaryOperation$Add$.class
BinaryOperation$Divide$.class
BinaryOperation$LogN$.class
BinaryOperation$Max$.class
BinaryOperation$Min$.class
BinaryOperation$Multiply$.class
BinaryOperation$Pow$.class
BinaryOperation$Remainder$.class
BinaryOperation$Subtract$.class
BinaryOperation.class

This PR makes it so that we'll search in the jvm for ml.combust.mleap.core.feature.BinaryOperation$Add$ instead of the current ml.combust.mleap.core.feature.BinaryOperation.Add$. To be honest I'm not sure why the current code doesn't break the existing tests.

Release Note

I have this PR based off of the v0.21.0 tag and intend to release this as v0.21.1. I'll also make a PR to apply this commit to master branch (which will later be released as v0.22.0)

@jsleight jsleight requested a review from austinzh December 22, 2022 15:19
Copy link
Contributor

@austinzh austinzh left a comment

Choose a reason for hiding this comment

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

LGTM.
We should create a release branch first thought

@jsleight jsleight changed the base branch from master to release/0.21.1 December 22, 2022 20:13
@jsleight jsleight merged commit ddbffa5 into release/0.21.1 Dec 22, 2022
@jsleight jsleight deleted the u/jsleight/backport_jvm11_nested_classes branch December 22, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants