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

Add public class members with the parameter names in each SGObject class #5042

Open
gf712 opened this issue May 21, 2020 · 7 comments
Open

Comments

@gf712
Copy link
Member

gf712 commented May 21, 2020

Currently we register parameter names using SG_ADD(&class_member, "name", "parameter description"). The problem is that changing the registered name (in this case "name") results in issues everytime we refactor the parameter names. To address this issue the class should register a static constexpr std::string_view kParam = "name" that should be used to register the class name.
If we ever need to use get/put we should use these constexpr string_views, to avoid any typos.

Example:

Parameter name is define here:

public:
static constexpr std::string_view kWeights = "weights";

And is used in parameter registration here:

SG_ADD(&m_weights, kWeights, "weights");

@gf712 gf712 assigned gf712 and unassigned gf712 May 21, 2020
JasonSurendran added a commit to JasonSurendran/shogun that referenced this issue Jul 9, 2020
karlnapf pushed a commit that referenced this issue Jul 15, 2020
* class members with the parameter names (#5042)

* WeightedMajorityVote, Clustering, CrossValidation

* GradientEval, MachineEval, MulticlassOVRE

* SigmoidCalibra, SplittingStrat, TimeSeries

* Cross Validation fix

* removing m prefix
@justdvnsh
Copy link

Hi @gf712 . I am new to Shogun. I'd like to start contributing to shogun with this issue. Can I be assigned this issue ?

@gf712
Copy link
Member Author

gf712 commented Oct 4, 2020

hi @justdvnsh, go for it!

@yiransii
Copy link

yiransii commented Nov 9, 2020

Hello! Is this issue still open? If so I would like to work on this issue.
I looked at the latest codebase, and it seems that some of the SG_ADD methods were still using strings rather than static variables.

@yiransii
Copy link

yiransii commented Nov 9, 2020

^following my previous comment

If so, could you tell me what places/models need to have a static variable defined as in the example? Thanks!

@jaskiratsingh2000
Copy link

@gf712 I am just a beginner in the coding world but holds a good knowledge of C++ which is required here and am super interested to work on this issue. can this issue be assigned to me? That would really be great.

@jaskiratsingh2000
Copy link

@gf712 Please please assign me on this. That would really be helpful. Thanks!

@Ishvina
Copy link

Ishvina commented Jan 20, 2022

Hi! I am in an intro to swe class that is requiring me to work on my first open-source project. I was wondering if this issue is still open and if so could I get help with my first contribution?

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

No branches or pull requests

5 participants