Skip to content

Make Feature class public instead hide it's ctor#45

Merged
tovbinm merged 2 commits intomasterfrom
mt/public-feature
Aug 9, 2018
Merged

Make Feature class public instead hide it's ctor#45
tovbinm merged 2 commits intomasterfrom
mt/public-feature

Conversation

@tovbinm
Copy link
Copy Markdown
Collaborator

@tovbinm tovbinm commented Aug 9, 2018

Related issues
Impossible to change the internal feature fields after creation, e.g response/predictor.

Describe the proposed solution
Making Feature case class public to allow changing feature fields if needed using .copy method. For example if one would like to change the type of the feature from response to predictor.
We still maintain the ctor as private, enforcing usage of creating features through FeatureBuilder.

Describe alternatives you've considered
None

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 9, 2018

Codecov Report

Merging #45 into master will increase coverage by 0.42%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #45      +/-   ##
=========================================
+ Coverage   83.47%   83.9%   +0.42%     
=========================================
  Files         296     296              
  Lines        9380    9679     +299     
  Branches      344     544     +200     
=========================================
+ Hits         7830    8121     +291     
- Misses       1550    1558       +8
Impacted Files Coverage Δ
...ain/scala/com/salesforce/op/features/Feature.scala 40% <ø> (ø) ⬆️
...cala/com/salesforce/op/utils/io/csv/CSVInOut.scala 0% <0%> (-100%) ⬇️
...es/src/main/scala/com/salesforce/op/OpParams.scala 89.58% <0%> (+4.16%) ⬆️
...a/com/salesforce/op/testkit/PartiallyDefined.scala 100% <0%> (+100%) ⬆️
...om/salesforce/op/testkit/FeatureFactoryOwner.scala 100% <0%> (+100%) ⬆️
...in/scala/com/salesforce/op/testkit/RandomSet.scala 66.66% <0%> (+66.66%) ⬆️
...com/salesforce/op/testkit/StandardRandomData.scala 100% <0%> (+100%) ⬆️
...ala/com/salesforce/op/testkit/InfiniteStream.scala 100% <0%> (+100%) ⬆️
...n/scala/com/salesforce/op/testkit/RandomData.scala 100% <0%> (+100%) ⬆️
...ala/com/salesforce/op/testkit/RandomIntegral.scala 100% <0%> (+100%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2086e5...2bf4fba. Read the comment docs.

@leahmcguire
Copy link
Copy Markdown
Collaborator

should we make FeatureLike private to avoid confusion?

@tovbinm tovbinm merged commit ae39758 into master Aug 9, 2018
@tovbinm tovbinm deleted the mt/public-feature branch August 9, 2018 21:02
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.

2 participants