Skip to content

Commit 3c43f0c

Browse files
effigieseffigies
authored
Merge pull request #2363 from oesteban/fix/displaynumber
[FIX] Errors parsing ``$DISPLAY``
2 parents f3b0912 + 603ae77 commit 3c43f0c

File tree

3 files changed

+53
-15
lines changed

3 files changed

+53
-15
lines changed

‎CHANGES‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
Upcoming release (0.14.1)
22
=========================
33

4+
* FIX: Errors parsing ``$DISPLAY`` (https://github.com/nipy/nipype/pull/2363)
45
* FIX: MultiProc starting workers at dubious wd (https://github.com/nipy/nipype/pull/2368)
56
* REF+FIX: Move BIDSDataGrabber to `interfaces.io` + fix correct default behavior (https://github.com/nipy/nipype/pull/2336)
67
* ENH: Add AFNI interface for 3dConvertDset (https://github.com/nipy/nipype/pull/2337)

‎nipype/utils/config.py‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,11 @@ def get_display(self):
312312
def _mock():
313313
pass
314314

315-
# Store a fake Xvfb object
316-
ndisp = int(sysdisplay.split(':')[-1])
315+
# Store a fake Xvfb object. Format - <host>:<display>[.<screen>]
316+
ndisp = sysdisplay.split(':')[-1].split('.')[0]
317317
Xvfb = namedtuple('Xvfb', ['new_display', 'stop'])
318-
self._display = Xvfb(ndisp, _mock)
319-
return sysdisplay
318+
self._display = Xvfb(int(ndisp), _mock)
319+
return self.get_display()
320320
else:
321321
if 'darwin' in sys.platform:
322322
raise RuntimeError(
@@ -343,8 +343,7 @@ def _mock():
343343
if not hasattr(self._display, 'new_display'):
344344
setattr(self._display, 'new_display',
345345
self._display.vdisplay_num)
346-
347-
return ':%d' % self._display.new_display
346+
return self.get_display()
348347

349348
def stop_display(self):
350349
"""Closes the display if started"""

‎nipype/utils/tests/test_config.py‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@
2828
spec=['vdisplay_num', 'start', 'stop'], vdisplay_num=2010)
2929

3030

31+
@pytest.mark.parametrize('dispvar', [':12', 'localhost:12', 'localhost:12.1'])
32+
def test_display_parse(monkeypatch, dispvar):
33+
"""Check that when $DISPLAY is defined, the display is correctly parsed"""
34+
config._display = None
35+
config._config.remove_option('execution', 'display_variable')
36+
monkeypatch.setenv('DISPLAY', dispvar)
37+
assert config.get_display() == ':12'
38+
# Test that it was correctly cached
39+
assert config.get_display() == ':12'
40+
41+
3142
@pytest.mark.parametrize('dispnum', range(5))
3243
def test_display_config(monkeypatch, dispnum):
3344
"""Check that the display_variable option is used ($DISPLAY not set)"""
@@ -46,7 +57,7 @@ def test_display_system(monkeypatch, dispnum):
4657
config._display = None
4758
config._config.remove_option('execution', 'display_variable')
4859
dispstr = ':%d' % dispnum
49-
monkeypatch.setitem(os.environ, 'DISPLAY', dispstr)
60+
monkeypatch.setenv('DISPLAY', dispstr)
5061
assert config.get_display() == dispstr
5162
# Test that it was correctly cached
5263
assert config.get_display() == dispstr
@@ -58,7 +69,7 @@ def test_display_config_and_system(monkeypatch):
5869
config._display = None
5970
dispstr = ':10'
6071
config.set('execution', 'display_variable', dispstr)
61-
monkeypatch.setitem(os.environ, 'DISPLAY', ':0')
72+
monkeypatch.setenv('DISPLAY', ':0')
6273
assert config.get_display() == dispstr
6374
# Test that it was correctly cached
6475
assert config.get_display() == dispstr
@@ -72,10 +83,17 @@ def test_display_noconfig_nosystem_patched(monkeypatch):
7283
config._config.remove_option('execution', 'display_variable')
7384
monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
7485
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch)
86+
monkeypatch.setattr(sys, 'platform', value='linux')
7587
assert config.get_display() == ":2010"
7688
# Test that it was correctly cached
7789
assert config.get_display() == ':2010'
7890

91+
# Check that raises in Mac
92+
config._display = None
93+
monkeypatch.setattr(sys, 'platform', value='darwin')
94+
with pytest.raises(RuntimeError):
95+
config.get_display()
96+
7997

8098
def test_display_empty_patched(monkeypatch):
8199
"""
@@ -85,12 +103,18 @@ def test_display_empty_patched(monkeypatch):
85103
config._display = None
86104
if config.has_option('execution', 'display_variable'):
87105
config._config.remove_option('execution', 'display_variable')
88-
monkeypatch.setitem(os.environ, 'DISPLAY', '')
106+
monkeypatch.setenv('DISPLAY', '')
89107
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch)
108+
monkeypatch.setattr(sys, 'platform', value='linux')
90109
assert config.get_display() == ':2010'
91110
# Test that it was correctly cached
92111
assert config.get_display() == ':2010'
93112

113+
# Check that raises in Mac
114+
config._display = None
115+
monkeypatch.setattr(sys, 'platform', value='darwin')
116+
with pytest.raises(RuntimeError):
117+
config.get_display()
94118

95119
def test_display_noconfig_nosystem_patched_oldxvfbwrapper(monkeypatch):
96120
"""
@@ -102,10 +126,16 @@ def test_display_noconfig_nosystem_patched_oldxvfbwrapper(monkeypatch):
102126
config._config.remove_option('execution', 'display_variable')
103127
monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
104128
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch_old)
129+
monkeypatch.setattr(sys, 'platform', value='linux')
105130
assert config.get_display() == ":2010"
106131
# Test that it was correctly cached
107132
assert config.get_display() == ':2010'
108133

134+
# Check that raises in Mac
135+
config._display = None
136+
monkeypatch.setattr(sys, 'platform', value='darwin')
137+
with pytest.raises(RuntimeError):
138+
config.get_display()
109139

110140
def test_display_empty_patched_oldxvfbwrapper(monkeypatch):
111141
"""
@@ -115,12 +145,18 @@ def test_display_empty_patched_oldxvfbwrapper(monkeypatch):
115145
config._display = None
116146
if config.has_option('execution', 'display_variable'):
117147
config._config.remove_option('execution', 'display_variable')
118-
monkeypatch.setitem(os.environ, 'DISPLAY', '')
148+
monkeypatch.setenv('DISPLAY', '')
119149
monkeypatch.setitem(sys.modules, 'xvfbwrapper', xvfbpatch_old)
150+
monkeypatch.setattr(sys, 'platform', value='linux')
120151
assert config.get_display() == ':2010'
121152
# Test that it was correctly cached
122153
assert config.get_display() == ':2010'
123154

155+
# Check that raises in Mac
156+
config._display = None
157+
monkeypatch.setattr(sys, 'platform', value='darwin')
158+
with pytest.raises(RuntimeError):
159+
config.get_display()
124160

125161
def test_display_noconfig_nosystem_notinstalled(monkeypatch):
126162
"""
@@ -130,7 +166,7 @@ def test_display_noconfig_nosystem_notinstalled(monkeypatch):
130166
config._display = None
131167
if config.has_option('execution', 'display_variable'):
132168
config._config.remove_option('execution', 'display_variable')
133-
monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
169+
monkeypatch.delenv('DISPLAY', raising=False)
134170
monkeypatch.setitem(sys.modules, 'xvfbwrapper', None)
135171
with pytest.raises(RuntimeError):
136172
config.get_display()
@@ -144,13 +180,14 @@ def test_display_empty_notinstalled(monkeypatch):
144180
config._display = None
145181
if config.has_option('execution', 'display_variable'):
146182
config._config.remove_option('execution', 'display_variable')
147-
monkeypatch.setitem(os.environ, 'DISPLAY', '')
183+
monkeypatch.setenv('DISPLAY', '')
148184
monkeypatch.setitem(sys.modules, 'xvfbwrapper', None)
149185
with pytest.raises(RuntimeError):
150186
config.get_display()
151187

152188

153189
@pytest.mark.skipif(not has_Xvfb, reason='xvfbwrapper not installed')
190+
@pytest.mark.skipif('darwin' in sys.platform, reason='macosx requires root for Xvfb')
154191
def test_display_noconfig_nosystem_installed(monkeypatch):
155192
"""
156193
Check that actually uses xvfbwrapper when installed (not mocked)
@@ -159,14 +196,15 @@ def test_display_noconfig_nosystem_installed(monkeypatch):
159196
config._display = None
160197
if config.has_option('execution', 'display_variable'):
161198
config._config.remove_option('execution', 'display_variable')
162-
monkeypatch.delitem(os.environ, 'DISPLAY', raising=False)
199+
monkeypatch.delenv('DISPLAY', raising=False)
163200
newdisp = config.get_display()
164201
assert int(newdisp.split(':')[-1]) > 1000
165202
# Test that it was correctly cached
166203
assert config.get_display() == newdisp
167204

168205

169206
@pytest.mark.skipif(not has_Xvfb, reason='xvfbwrapper not installed')
207+
@pytest.mark.skipif('darwin' in sys.platform, reason='macosx requires root for Xvfb')
170208
def test_display_empty_installed(monkeypatch):
171209
"""
172210
Check that actually uses xvfbwrapper when installed (not mocked)
@@ -175,7 +213,7 @@ def test_display_empty_installed(monkeypatch):
175213
config._display = None
176214
if config.has_option('execution', 'display_variable'):
177215
config._config.remove_option('execution', 'display_variable')
178-
monkeypatch.setitem(os.environ, 'DISPLAY', '')
216+
monkeypatch.setenv('DISPLAY', '')
179217
newdisp = config.get_display()
180218
assert int(newdisp.split(':')[-1]) > 1000
181219
# Test that it was correctly cached
@@ -191,7 +229,7 @@ def test_display_empty_macosx(monkeypatch):
191229
config._display = None
192230
if config.has_option('execution', 'display_variable'):
193231
config._config.remove_option('execution', 'display_variable')
194-
monkeypatch.delitem(os.environ, 'DISPLAY', '')
232+
monkeypatch.delenv('DISPLAY', '')
195233

196234
monkeypatch.setattr(sys, 'platform', 'darwin')
197235
with pytest.raises(RuntimeError):

0 commit comments

Comments
 (0)