Ticket #3870 (closed: fixed)

Opened 9 years ago

Last modified 5 years ago

Check adherence to algorithm property naming conventions

Reported by: Russell Taylor Owned by: Karl Palmen
Priority: critical Milestone: Release 2.0
Component: Mantid Keywords:
Cc: Blocked By:
Blocking: Tester: Nick Draper

Description (last modified by Nick Draper) (diff)

As described at http://www.mantidproject.org/Mantid_Standards

Correct deviations from this.
In particular, there are places where an underscore is being used. At the time of writing this happens (as far as I could find) in:

  • LoadEventNexus
  • SumRowColumn
  • SaveNXSPE
  • ConvertToQ3DdE
  • SimulateMDD

A full list has been attached in the txt file on this ticket

Once you are finished send an email with all of the changes ot Nick Draper for inclusion in the next release notes.

Attachments

Wrong_Alg_Names.txt (2.3 KB) - added by Nick Draper 9 years ago.
AlgorithmPropertyNameChanges.txt (1.3 KB) - added by Karl Palmen 9 years ago.
Changes to property names I have made for this ticket.

Change History

comment:1 Changed 9 years ago by Russell Taylor

  • Summary changed from Check adherence to property naming conventions to Check adherence to algorithm property naming conventions

comment:2 Changed 9 years ago by Russell Taylor

A duplicate ticket (#3877) was created under which the LoadEventNexus names were fixed. The others remain.

Changed 9 years ago by Nick Draper

comment:3 Changed 9 years ago by Nick Draper

  • Owner set to Karl Palmen
  • Status changed from new to assigned
  • Description modified (diff)

comment:4 Changed 9 years ago by Karl Palmen

Should the convention apply to properties that are referred to in the outside world by a lower-case character in either the Latin or Greek alphabets?

Forcing such properties to conform to begin with capital letters could confuse or annoy customers. If such properties are not ammended, then the conventions need ammending to include such an exception.

comment:5 Changed 9 years ago by Karl Palmen

The properties of LoadEventNexus appear to have been fixed as documented in http://www.mantidproject.org/LoadEventNexus

comment:6 Changed 9 years ago by Karl Palmen

There is no reference to any properties in LoadSNSEventNexus. It just calls LoadEventNexus.

comment:7 Changed 9 years ago by Karl Palmen

  • Status changed from assigned to accepted

comment:8 Changed 9 years ago by Karl Palmen

According to the wiki http://www.mantidproject.org/CatalogMyDataSearch , CatalogMyDataSearch is no "isValid" property and has code in login.h/.cpp . According to the code I have, an algorithm cMyDataSearch does have such a property in MyDataSearch.cpp .

comment:9 Changed 9 years ago by Karl Palmen

kernel_exports.cpp lists a large number of lower case properties missed out of attachment Wrong_Alg_Names.txt .

Changed 9 years ago by Karl Palmen

Changes to property names I have made for this ticket.

comment:10 Changed 9 years ago by Karl Palmen

Changed properties as specified in recent attachment.

The (a, b, c, alpha, beta, gamma) properties are unchanged because of algorithm convention.

Property inFilename not found in code or wiki.

comment:11 Changed 9 years ago by Karl Palmen

  • Status changed from accepted to verify
  • Resolution set to fixed

comment:12 Changed 9 years ago by Karl Palmen

local commit to enable pull re #3870

Changeset: b5eaa5a25006913125b4bd576481e34eb8ccb710

comment:13 Changed 9 years ago by Karl Palmen

Changed algorithm properties and specified in recently attached file re #3870

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: e2a2059c276912e1a63813700a7fa0649bee663f

comment:14 Changed 9 years ago by Andrei Savici

Note: check if any of the changes affect the scripts shipped with Mantid. See for example the change in Changeset: fabd7dc9b224470af899808c8f207282aaa74dc0, for ticket #3208.

comment:15 Changed 9 years ago by Karl Palmen

I've made such checks with Astrogrep and find no others besides the one you changed, except also "psi=" needs changing to "Psi=" in the line you changed.

comment:16 Changed 9 years ago by Karl Palmen

Update property names in python files re #3870

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 7f0d204ee69f20bcf7d4b293726143e61a345bcd

comment:17 Changed 9 years ago by Karl Palmen

Update property names in python files re #3870

Signed-off-by: Karl Palmen <karl.palmen@…>

Changeset: 7f0d204ee69f20bcf7d4b293726143e61a345bcd

comment:18 Changed 9 years ago by Nick Draper

  • Status changed from verify to verifying
  • Tester set to Nick Draper

comment:19 Changed 9 years ago by Nick Draper

  • Status changed from verifying to closed

tested using this script:

import re

def RemoveVersion(algName):
	m = re.search('\w+', algName)
	return m.group(0)
	
def FirstCapital(algName):
	m = re.search('^[A-Z].*', algName)
	if m is None:
		return "The first letter must be a capital.\n"
	else:
		return ""
		
def IllegalCaracters(algName):
	m = re.search('[\-\_\(\)\[\]]', algName)
	if m is None:
		return ""
	else:
		return "Contains illegal characters.\n"
	

with open('c:/mantid/algnames.txt', 'r') as f:
	algNames = f.readlines()
f.closed

for algName in algNames:
	errorMessage=""
	algName = RemoveVersion(algName)
	errorMessage+=FirstCapital(algName)
	errorMessage+=IllegalCaracters(algName)
	alg=mtd.createAlgorithm(algName)
	if errorMessage <>"": print algName, errorMessage
	for prop in alg.getProperties():
		propError = ""
		propError+=FirstCapital(algName)
		propError+=IllegalCaracters(algName)
		if propError <>"": print algName, prop.name, propError

comment:20 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 4717

Note: See TracTickets for help on using tickets.