From c6f0a7a4847cc0db8f5cdf7cf286009d3854754b Mon Sep 17 00:00:00 2001 From: Todd Date: Fri, 26 Mar 2021 14:49:09 +0000 Subject: [PATCH 1/7] RF: Modularise XML saving - each component, routine, etc. has an xml method instead of one big function in experiment --- psychopy/experiment/_experiment.py | 66 ++++++------------- psychopy/experiment/components/_base.py | 19 ++++++ .../components/settings/__init__.py | 15 +++++ psychopy/experiment/flow.py | 16 +++++ psychopy/experiment/loops.py | 26 ++++++++ psychopy/experiment/routine.py | 12 ++++ 6 files changed, 109 insertions(+), 45 deletions(-) diff --git a/psychopy/experiment/_experiment.py b/psychopy/experiment/_experiment.py index 4442d174b17..ab92cd3bcc0 100644 --- a/psychopy/experiment/_experiment.py +++ b/psychopy/experiment/_experiment.py @@ -276,54 +276,30 @@ def writeScript(self, expPath=None, target="PsychoPy", modular=True): return script + @property + def xml(self): + # Create experiment root element + experimentNode = xml.Element("PsychoPy2experiment") + experimentNode.set('version', __version__) + experimentNode.set('encoding', 'utf-8') + # Add settings node + settingsNode = self.settings.xml + experimentNode.append(settingsNode) + # Add routines node + routineNode = xml.Element("Routines") + for key, routine in self.routines.items(): + routineNode.append(routine.xml) + experimentNode.append(routineNode) + # Add flow node + flowNode = self.flow.xml + experimentNode.append(flowNode) + + return experimentNode + def saveToXML(self, filename): self.psychopyVersion = psychopy.__version__ # make sure is current # create the dom object - self.xmlRoot = xml.Element("PsychoPy2experiment") - self.xmlRoot.set('version', __version__) - self.xmlRoot.set('encoding', 'utf-8') - # store settings - settingsNode = xml.SubElement(self.xmlRoot, 'Settings') - for settingName in sorted(self.settings.params): - setting = self.settings.params[settingName] - self._setXMLparam( - parent=settingsNode, param=setting, name=settingName) - # store routines - routinesNode = xml.SubElement(self.xmlRoot, 'Routines') - # routines is a dict of routines - for routineName, routine in self.routines.items(): - routineNode = self._setXMLparam( - parent=routinesNode, param=routine, name=routineName) - # a routine is based on a list of components - for component in routine: - componentNode = self._setXMLparam( - parent=routineNode, param=component, - name=component.params['name'].val) - for paramName in sorted(component.params): - param = component.params[paramName] - self._setXMLparam( - parent=componentNode, param=param, name=paramName) - # implement flow - flowNode = xml.SubElement(self.xmlRoot, 'Flow') - # a list of elements(routines and loopInit/Terms) - for element in self.flow: - elementNode = xml.SubElement(flowNode, element.getType()) - if element.getType() == 'LoopInitiator': - loop = element.loop - name = loop.params['name'].val - elementNode.set('loopType', loop.getType()) - elementNode.set('name', name) - for paramName in sorted(loop.params): - param = loop.params[paramName] - paramNode = self._setXMLparam( - parent=elementNode, param=param, name=paramName) - # override val with repr(val) - if paramName == 'conditions': - paramNode.set('val', repr(param.val)) - elif element.getType() == 'LoopTerminator': - elementNode.set('name', element.loop.params['name'].val) - elif element.getType() == 'Routine': - elementNode.set('name', '%s' % element.params['name']) + self.xmlRoot = self.xml # convert to a pretty string # update our document to use the new root self._doc._setroot(self.xmlRoot) diff --git a/psychopy/experiment/components/_base.py b/psychopy/experiment/components/_base.py index 09076e55baa..d842e719808 100644 --- a/psychopy/experiment/components/_base.py +++ b/psychopy/experiment/components/_base.py @@ -12,6 +12,7 @@ from builtins import str, object, super from past.builtins import basestring from pathlib import Path +from xml.etree.ElementTree import Element from psychopy import prefs from psychopy.constants import FOREVER @@ -124,6 +125,24 @@ def __init__(self, exp, parentName, name='', self.order = ['name'] # name first, then timing, then others + @property + def xml(self): + # Make root element + element = Element(self.__class__.__name__) + element.set("name", self.params['name'].val) + # Add an element for each parameter + for key, param in self.params.items(): + if key == 'name': + continue + paramNode = Element("Param") + paramNode.set("name", key) + paramNode.set("updates", str(param.updates)) + paramNode.set("val", str(param.val).replace("\n", " ")) + paramNode.set("valType", str(param.valType)) + element.append(paramNode) + + return element + def integrityCheck(self): """ Run component integrity checks for non-visual components diff --git a/psychopy/experiment/components/settings/__init__.py b/psychopy/experiment/components/settings/__init__.py index 4c2aa36585e..da67369880c 100644 --- a/psychopy/experiment/components/settings/__init__.py +++ b/psychopy/experiment/components/settings/__init__.py @@ -7,6 +7,7 @@ from builtins import object import os from pathlib import Path +from xml.etree.ElementTree import Element import re import wx.__version__ import psychopy @@ -315,6 +316,20 @@ def __init__(self, parentName, exp, expName='', fullScr=True, hint=_translate("When to export experiment to the HTML folder."), label=_localized["Export HTML"], categ='Online') + @property + def xml(self): + # Make root element + element = Element("Settings") + # Add an eleent for each parameter + for key, param in self.params.items(): + paramNode = Element("Param") + paramNode.set("name", key) + paramNode.set("updates", str(param.updates)) + paramNode.set("val", str(param.val).replace("\n", " ")) + paramNode.set("valType", str(param.valType)) + element.append(paramNode) + return element + def getInfo(self): """Rather than converting the value of params['Experiment Info'] into a dict from a string (which can lead to errors) use this function diff --git a/psychopy/experiment/flow.py b/psychopy/experiment/flow.py index 11e092cffbf..88af71052c5 100644 --- a/psychopy/experiment/flow.py +++ b/psychopy/experiment/flow.py @@ -10,6 +10,7 @@ from __future__ import absolute_import, print_function from past.builtins import basestring +from xml.etree.ElementTree import Element from psychopy.experiment.routine import Routine from psychopy.experiment.loops import LoopTerminator, LoopInitiator @@ -54,6 +55,21 @@ def loopDict(self): def __repr__(self): return "psychopy.experiment.Flow(%s)" % (str(list(self))) + @property + def xml(self): + # Make root element + element = Element("Flow") + # Add an element for every Routine, Loop Initiator, Loop Terminator + for item in self: + sub = item.xml + if isinstance(item, Routine): + # Remove all sub elements (we only need its name) + for comp in sub: + sub.remove(comp) + element.append(sub) + + return element + def addLoop(self, loop, startPos, endPos): """Adds initiator and terminator objects for the loop into the Flow list""" diff --git a/psychopy/experiment/loops.py b/psychopy/experiment/loops.py index 9010c958f54..17cc78998f4 100755 --- a/psychopy/experiment/loops.py +++ b/psychopy/experiment/loops.py @@ -18,6 +18,8 @@ from __future__ import absolute_import, print_function from builtins import object +from xml.etree.ElementTree import Element + # from future import standard_library from psychopy.experiment import getInitVals @@ -592,6 +594,22 @@ def __init__(self, loop): self.exp = loop.exp loop.initiator = self + @property + def xml(self): + # Make root element + element = Element("LoopInitiator") + element.set("loopType", self.loop.__class__.__name__) + element.set("name", self.loop.params['name'].val) + # Add an element for each parameter + for key, param in self.loop.params.items(): + paramNode = Element("Param") + paramNode.set("name", key) + paramNode.set("updates", str(param.updates)) + paramNode.set("val", str(param.val).replace("\n", " ")) + paramNode.set("valType", str(param.valType)) + element.append(paramNode) + return element + def getType(self): return 'LoopInitiator' @@ -625,6 +643,14 @@ def __init__(self, loop): self.exp = loop.exp loop.terminator = self + @property + def xml(self): + # Make root element + element = Element("LoopTerminator") + element.set("name", self.loop.params['name'].val) + + return element + def getType(self): return 'LoopTerminator' diff --git a/psychopy/experiment/routine.py b/psychopy/experiment/routine.py index 25440b16ea2..2640d989ea6 100644 --- a/psychopy/experiment/routine.py +++ b/psychopy/experiment/routine.py @@ -11,6 +11,7 @@ from __future__ import absolute_import, print_function from psychopy.constants import FOREVER +from xml.etree.ElementTree import Element class Routine(list): @@ -37,6 +38,17 @@ def __repr__(self): _rep = "psychopy.experiment.Routine(name='%s', exp=%s, components=%s)" return _rep % (self.name, self.exp, str(list(self))) + @property + def xml(self): + # Make root element + element = Element("Routine") + element.set("name", self.name) + # Add each component's element + for comp in self: + element.append(comp.xml) + + return element + @property def name(self): return self.params['name'] From 7e1a71c8da97969a2b280be40144ad9c5b15f92b Mon Sep 17 00:00:00 2001 From: Todd Date: Mon, 29 Mar 2021 09:18:26 +0100 Subject: [PATCH 2/7] FF: Small adjustments to param order to bring into line with old xml writing --- psychopy/experiment/components/_base.py | 15 +++++++++------ .../experiment/components/settings/__init__.py | 17 ++++++++++++----- psychopy/experiment/loops.py | 13 +++++++++---- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/psychopy/experiment/components/_base.py b/psychopy/experiment/components/_base.py index d842e719808..b490d45161a 100644 --- a/psychopy/experiment/components/_base.py +++ b/psychopy/experiment/components/_base.py @@ -131,14 +131,17 @@ def xml(self): element = Element(self.__class__.__name__) element.set("name", self.params['name'].val) # Add an element for each parameter - for key, param in self.params.items(): - if key == 'name': - continue + for key, param in sorted(self.params.items()): + # Create node paramNode = Element("Param") paramNode.set("name", key) - paramNode.set("updates", str(param.updates)) - paramNode.set("val", str(param.val).replace("\n", " ")) - paramNode.set("valType", str(param.valType)) + # Assign values + if hasattr(param, 'updates'): + paramNode.set('updates', "{}".format(param.updates)) + if hasattr(param, 'val'): + paramNode.set('val', u"{}".format(param.val).replace("\n", " ")) + if hasattr(param, 'valType'): + paramNode.set('valType', param.valType) element.append(paramNode) return element diff --git a/psychopy/experiment/components/settings/__init__.py b/psychopy/experiment/components/settings/__init__.py index da67369880c..aa35098647b 100644 --- a/psychopy/experiment/components/settings/__init__.py +++ b/psychopy/experiment/components/settings/__init__.py @@ -320,13 +320,20 @@ def __init__(self, parentName, exp, expName='', fullScr=True, def xml(self): # Make root element element = Element("Settings") - # Add an eleent for each parameter - for key, param in self.params.items(): + # Add an element for each parameter + for key, param in sorted(self.params.items()): + if key == 'name': + continue + # Create node paramNode = Element("Param") paramNode.set("name", key) - paramNode.set("updates", str(param.updates)) - paramNode.set("val", str(param.val).replace("\n", " ")) - paramNode.set("valType", str(param.valType)) + # Assign values + if hasattr(param, 'updates'): + paramNode.set('updates', "{}".format(param.updates)) + if hasattr(param, 'val'): + paramNode.set('val', u"{}".format(param.val).replace("\n", " ")) + if hasattr(param, 'valType'): + paramNode.set('valType', param.valType) element.append(paramNode) return element diff --git a/psychopy/experiment/loops.py b/psychopy/experiment/loops.py index 17cc78998f4..1d47298092d 100755 --- a/psychopy/experiment/loops.py +++ b/psychopy/experiment/loops.py @@ -601,12 +601,17 @@ def xml(self): element.set("loopType", self.loop.__class__.__name__) element.set("name", self.loop.params['name'].val) # Add an element for each parameter - for key, param in self.loop.params.items(): + for key, param in sorted(self.loop.params.items()): + # Create node paramNode = Element("Param") paramNode.set("name", key) - paramNode.set("updates", str(param.updates)) - paramNode.set("val", str(param.val).replace("\n", " ")) - paramNode.set("valType", str(param.valType)) + # Assign values + if hasattr(param, 'updates'): + paramNode.set('updates', "{}".format(param.updates)) + if hasattr(param, 'val'): + paramNode.set('val', u"{}".format(param.val).replace("\n", " ")) + if hasattr(param, 'valType'): + paramNode.set('valType', param.valType) element.append(paramNode) return element From 67ba1d16dd7f0c4547e1303ab964b3f78ee4f3ce Mon Sep 17 00:00:00 2001 From: Todd Date: Mon, 29 Mar 2021 10:53:39 +0100 Subject: [PATCH 3/7] FF: Copy list of components in routine before iteratively deleting them, to prevent list size changing each iteration --- psychopy/experiment/flow.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/psychopy/experiment/flow.py b/psychopy/experiment/flow.py index 88af71052c5..1454c51c626 100644 --- a/psychopy/experiment/flow.py +++ b/psychopy/experiment/flow.py @@ -64,7 +64,8 @@ def xml(self): sub = item.xml if isinstance(item, Routine): # Remove all sub elements (we only need its name) - for comp in sub: + comps = [comp for comp in sub] + for comp in comps: sub.remove(comp) element.append(sub) From 50699888b9fc0d5494aa5136ac9e970f1ba79957 Mon Sep 17 00:00:00 2001 From: Todd Date: Mon, 29 Mar 2021 11:01:40 +0100 Subject: [PATCH 4/7] FF: Set encoding before version as in old method --- psychopy/experiment/_experiment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/psychopy/experiment/_experiment.py b/psychopy/experiment/_experiment.py index ab92cd3bcc0..24387e31808 100644 --- a/psychopy/experiment/_experiment.py +++ b/psychopy/experiment/_experiment.py @@ -280,8 +280,8 @@ def writeScript(self, expPath=None, target="PsychoPy", modular=True): def xml(self): # Create experiment root element experimentNode = xml.Element("PsychoPy2experiment") - experimentNode.set('version', __version__) experimentNode.set('encoding', 'utf-8') + experimentNode.set('version', __version__) # Add settings node settingsNode = self.settings.xml experimentNode.append(settingsNode) From 27314fd63a539a4b3a3a205ed87c05623368007b Mon Sep 17 00:00:00 2001 From: Todd Date: Mon, 29 Mar 2021 13:31:46 +0100 Subject: [PATCH 5/7] TEST: Add test that xml saving and loading don't change the file --- .../test_app/test_builder/test_Experiment.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/psychopy/tests/test_app/test_builder/test_Experiment.py b/psychopy/tests/test_app/test_builder/test_Experiment.py index fa1e3a0e729..3c77b2cf6c4 100644 --- a/psychopy/tests/test_app/test_builder/test_Experiment.py +++ b/psychopy/tests/test_app/test_builder/test_Experiment.py @@ -1,6 +1,8 @@ from __future__ import print_function from past.builtins import execfile from builtins import object +from pathlib import Path +import xml.etree.ElementTree as xml import psychopy.experiment from psychopy.experiment.components.text import TextComponent @@ -73,6 +75,25 @@ def setup(self): def teardown_class(cls): shutil.rmtree(cls.tmp_dir, ignore_errors=True) + def test_xml(self): + # Get all psyexp files in demos folder + demosFolder = Path(self.exp.prefsPaths['demos']) / 'builder' + for file in demosFolder.glob("**/*.psyexp"): + # Create experiment and load from psyexp + exp = psychopy.experiment.Experiment() + exp.loadFromXML(file) + # Compile to get what script should look like + target = exp.writeScript() + # Save as XML + temp = str(Path(self.tmp_dir) / "testXML.psyexp") + exp.saveToXML(temp) + # Load again + exp.loadFromXML(temp) + # Compile again + test = exp.writeScript() + # Compare two scripts to make sure saving and loading hasn't changed anything + assert target == test + def test_xsd(self): # get files From 69798cd077e57ee79e14359a2e9839c4f779e6df Mon Sep 17 00:00:00 2001 From: Todd Date: Wed, 31 Mar 2021 10:59:33 +0100 Subject: [PATCH 6/7] TEST: Use difflib rather than == to compare target and test scripts, giving more informative output --- psychopy/tests/test_app/test_builder/test_Experiment.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/psychopy/tests/test_app/test_builder/test_Experiment.py b/psychopy/tests/test_app/test_builder/test_Experiment.py index 3c77b2cf6c4..945deaf6ac7 100644 --- a/psychopy/tests/test_app/test_builder/test_Experiment.py +++ b/psychopy/tests/test_app/test_builder/test_Experiment.py @@ -92,7 +92,8 @@ def test_xml(self): # Compile again test = exp.writeScript() # Compare two scripts to make sure saving and loading hasn't changed anything - assert target == test + diff = difflib.unified_diff(target.splitlines(), test.splitlines()) + assert list(diff) == [] def test_xsd(self): # get files From cf05a1c0776b25282621e09c80a947d593af0acc Mon Sep 17 00:00:00 2001 From: Todd Date: Wed, 31 Mar 2021 11:21:46 +0100 Subject: [PATCH 7/7] TEST: Remove timestamps from compiled scripts when comparing, otherwise the line with the time on will be different if compile took >1s --- psychopy/tests/test_app/test_builder/test_Experiment.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/psychopy/tests/test_app/test_builder/test_Experiment.py b/psychopy/tests/test_app/test_builder/test_Experiment.py index 945deaf6ac7..5eed832f48a 100644 --- a/psychopy/tests/test_app/test_builder/test_Experiment.py +++ b/psychopy/tests/test_app/test_builder/test_Experiment.py @@ -20,6 +20,7 @@ from lxml import etree import numpy import sys +import re # Jeremy Gray March 2011 @@ -32,6 +33,7 @@ # load should change things allComponents = psychopy.experiment.getComponents(fetchIcons=False) +isTime = re.compile(r"\d+:\d+(:\d+)?( [AP]M)?") def _filterout_legal(lines): @@ -91,6 +93,9 @@ def test_xml(self): exp.loadFromXML(temp) # Compile again test = exp.writeScript() + # Remove any timestamps from script (these can cause false errors if compile takes longer than a second) + test = re.sub(isTime, "", test) + target = re.sub(isTime, "", target) # Compare two scripts to make sure saving and loading hasn't changed anything diff = difflib.unified_diff(target.splitlines(), test.splitlines()) assert list(diff) == []