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

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

Merged
merged 2 commits into from
Aug 9, 2018

Conversation

tovbinm
Copy link
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

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
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.

None yet

2 participants