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

Inconsistent casing in Lines and LineCodes for rmatrix, xmatrix and cmatrix #40

Open
kdheepak opened this issue Feb 17, 2019 · 2 comments

Comments

@kdheepak
Copy link
Member

kdheepak commented Feb 17, 2019

Currently the ODD interface exposes the following functions:

Lines.RMatrix
LineCodes.Rmatrix

Lines.XMatrix
LineCodes.Xmatrix

Lines.CMatrix
LineCodes.Cmatrix

It would be nice if these are consistent. @tshort, @PMeira any preferences?

The C API uses Lines_Get_Rmatrix, LineCodes_Get_Rmatrix, etc. So my vote is to use the same casing, i.e. change the functions in the Lines module. I believe the COM interface in the official distribution also uses this casing here.

The main reason I'm opening an issue for this is that this is technically an API breaking change, so it would require a major version update. But given that v0.5.0 hasn't been updated on the METADATA.jl yet, it might be safe to assume no one is using the tagged v0.5.0 version yet and maybe we can get away with the change in v0.5.1. Thoughts?

Alternatively, we can go with deprecation warnings and remove the other functions in the next major release.

@PMeira
Copy link
Member

PMeira commented Feb 18, 2019

The C API uses Lines_Get_Rmatrix, LineCodes_Get_Rmatrix, etc. So my vote is to use the same casing, i.e. change the functions in the Lines module. I believe the COM interface in the official distribution also uses this casing here.

Yeah, that's from COM. Since Pascal and lots of COM implementations treat the names as case insensitive, the original developers didn't pay much attention to this.

Personally I'd prefer things RMatrix but don't mind much -- the logic is that the last word is started with uppercase. Some "case transformers" I've seen in the past would translate that to "r_matrix" for snake case, not sure if Rmatrix would map well, but there are more harder examples like EmergVminpu. It varies a lot, e.g. BuildYMatrix, IntervalHrs, Total_Time, Rneut, Xhl.

We would need to review a lot of names. In general I prefer full names rather than shortened ones (besides the more obvious ones), I dislike things like Ckt when Circuit is also used.

I'm also in favor of updating the C header with a consistent naming convention to match this, we can expose deprecated aliases in the DLL for some time so it doesn't break compatibility out of nowhere.

The modules that mimic COM don't have much alternative at the moment -- if dss_sharp manages to work as well as the official DLL as a COM implementation, we can evaluate that in the long term.

@kdheepak
Copy link
Member Author

So it seems like a lot of the naming needs to be reviewed. Maybe we can make a full list of the changes needed and then deal with this issue.

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

No branches or pull requests

2 participants