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

Remove camel case from unified_power_Rg (Trac #1059) #288

Open
smk78 opened this issue Mar 30, 2019 · 9 comments
Open

Remove camel case from unified_power_Rg (Trac #1059) #288

smk78 opened this issue Mar 30, 2019 · 9 comments

Comments

@smk78
Copy link
Contributor

smk78 commented Mar 30, 2019

@llimeht points out in !SasView PR 137 that unified_power_Rg contains capitalisation (in contravention of the agreed coding rules) meaning links to the corresponding help documentation are not working.

Though the PR removes case dependency, the expedient approach is also to enforce the coding standard. Thus the model should be renamed and links to the doc pages checked.

Migrated from http://trac.sasview.org/ticket/1059

{
    "status": "reopened",
    "changetime": "2019-03-29T14:02:58",
    "_ts": "2019-03-29 14:02:58.829996+00:00",
    "description": "@llimeht points out in !SasView PR 137 that unified_power_Rg contains capitalisation (in contravention of the agreed coding rules) meaning links to the corresponding help documentation are not working.\n\nThough the PR removes case dependency, the expedient approach is also to enforce the coding standard. Thus the model should be renamed and links to the doc pages checked.",
    "reporter": "smk78",
    "cc": "",
    "resolution": "",
    "workpackage": "SasView Bug Fixing",
    "time": "2018-01-09T14:33:29",
    "component": "SasView",
    "summary": "Remove camel case from unified_power_Rg",
    "priority": "minor",
    "keywords": "",
    "milestone": "SasView 4.3.0",
    "owner": "butler",
    "type": "defect"
}
@pkienzle
Copy link
Contributor

Trac update at 2018/01/16 02:47:23: pkienzle commented:

The Rg in this case is not camel case since it is naturally capitalized and is preceded by an underscore. Much like we ignore the PEP 8 rules for case for variable names when matching the math symbols used in a paper, we could claim that this is a legitimate use of an upper case name for a model.

@butlerpd
Copy link
Member

Trac update at 2018/01/23 01:25:54:

  • butler changed owner from "" to "butler"
  • butler changed status from "new" to "assigned"

@butlerpd
Copy link
Member

Trac update at 2018/01/23 01:39:28: butler commented:

True but the rule was "no capitalization" not "no camel case". The later is a result of the first. Relevant section reads:

Please follow these new naming rules:

* No capitalization and thus no CamelCase. If necessary use underscore to separate (i.e. barbell not BarBell or broad_peak not BroadPeak)
* Remove “model” from the name (i.e. barbell not BarBellModel)

@butlerpd
Copy link
Member

Trac update at 2018/01/23 15:06:02: butler changed priority from "blocker" to "major"

@butlerpd
Copy link
Member

Trac update at 2018/04/02 22:31:44: butler changed priority from "major" to "minor"

@butlerpd
Copy link
Member

Trac update at 2018/06/15 14:54:37:

  • butler commented:

Probably cannot be done for this release so moving to 4.3

  • butler changed milestone from "SasView 4.2.0" to "SasView 4.3.0"

@smk78
Copy link
Contributor Author

smk78 commented Mar 30, 2019

Trac update at 2019/03/28 12:38:01:

  • smk78 commented:

In changeset fa8e6dc:

#!CommitTicketReference repository="sasmodels" revision="fa8e6dcf3e461c78e01fc9879eb87ba90aa8db08"
Changed unified_power_Rg into unified_power_rg. Closes #1115
  • smk78 changed resolution from "" to "fixed"
  • smk78 changed status from "assigned" to "closed"

@pkienzle
Copy link
Contributor

Trac update at 2019/03/29 00:10:56:

  • pkienzle commented:

Test whether a unified_power_Rg model saved from 4.0, 4.1 and 4.2 can be loaded into 4.3. Glancing at the conversion code I suspect the answer is no, but I haven't tested.

  • pkienzle changed resolution from "fixed" to ""
  • pkienzle changed status from "closed" to "reopened"

@smk78
Copy link
Contributor Author

smk78 commented Mar 30, 2019

Trac update at 2019/03/29 14:02:58: smk78 commented:

I have reverted the capitalisation changes in the Ticket 822 v5 unit tests branch (and not by reverting the original commit in master) so that the implications for old projects can be explored separately.

@ricleal ricleal transferred this issue from SasView/sasview Apr 23, 2019
@pkienzle pkienzle added this to the sasmodels Next Release +1 milestone Apr 24, 2019
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

3 participants