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.

Sign up for GitHub

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

Use os.path.join for file path handling #53

Open
wants to merge 2 commits into
base: experimental
Choose a base branch
from
+35 −69
Open

Use os.path.join for file path handling #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce08f11
Use os.path.join for file path handling
Oct 17, 2024
9488d8d
Refactor
Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions QDS.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import platform
from datetime import datetime
import QDSpy_checks # noqa: F401
import QDSpy_file_support as fsu
import QDSpy_global as glo
import QDSpy_stim as stm
import QDSpy_config as cfg
Expand Down Expand Up @@ -87,17 +86,12 @@ def Initialize(_sName="noname", _sDescr="nodescription", _runMode=1):
if os.path.isfile(fNameDir_pk):
tLastUpt_pick = datetime.fromtimestamp(os.path.getmtime(fNameDir_pk))
if tLastUpt_pick > tLastUpt_py and not args.compile:
pythonPath = fsu.getQDSpyPath()
if len(pythonPath) > 0:
pythonPath += "\\" if PLATFORM_WINDOWS else "/"

_log.Log.write("INFO", "Script has not changed, running stimulus now ...")
s = "python {0}QDSpy_core.py -t={1} {2} {3}"
os.system(s.format(
pythonPath if PLATFORM_WINDOWS else "",
args.timing, "-v" if args.verbose else "",
_Stim.fNameDir)
)
command = "python {0} -t={1} {2} {3}".format(
os.path.join(glo.QDSpy_path, "QDSpy_core.py"),
args.timing, "-v" if args.verbose else "",
_Stim.fNameDir)
os.system(command)
'''
os.system(s.format(
pythonPath if PLATFORM_WINDOWS else "",
Expand Down
3 changes: 1 addition & 2 deletions QDSpy_MQTT_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from collections import deque
import QDSpy_global as glo
import QDSpy_stim as stm
import QDSpy_file_support as fsu
from QDSpy_app import QDSpyApp, State, StateStr
import Libraries.mqtt_client as mqtt
import Libraries.mqtt_globals as mgl
Expand Down Expand Up @@ -115,7 +114,7 @@ def processMsg(self) -> None:
if self.state in [State.idle, State.ready]:
# Try loading the stimulus
# "load,<msg index>,<stimulus file name>"
fName = fsu.getQDSpyPath() +self.Conf.pathStim +msg[1][1]
fName = os.path.join(self.Conf.pathStim, msg[1][1])
errC = self.loadStim(fName)

elif msg[0] == mgl.Command.PLAY:
Expand Down
1 change: 0 additions & 1 deletion QDSpy_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ def log(self, _data):
def openLogFile(_fPath) -> TextIO:
"""Open a log file
"""
_fPath = fsu.repairPath(fsu.getQDSpyPath() +_fPath)
#print("openLogFile", _fPath)
os.makedirs(_fPath, exist_ok=True)
fName = time.strftime("%Y%m%d_%H%M%S")
Expand Down
3 changes: 1 addition & 2 deletions QDSpy_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ def __init__(self):
#
self.isWindows = PLATFORM_WINDOWS
self.pyVersion = sys.version_info[0] + sys.version_info[1] / 10
_sep = "\\" if PLATFORM_WINDOWS else "/"
self.iniPath = os.getcwd() + _sep + glo.QDSpy_iniFileName
self.iniPath = glo.QDSpy_iniFileName

# Set configuration default values
#
Expand Down
13 changes: 5 additions & 8 deletions QDSpy_core_presenter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import QDSpy_stim_video as vid
import QDSpy_stim_draw as drw
import QDSpy_core_support as csp
import QDSpy_file_support as fsu
import QDSpy_core_shader as csh
from Libraries.log_helper import Log
import Libraries.multiprocess_helper as mpr
Expand Down Expand Up @@ -68,8 +67,7 @@ def __init__(self, _Stage, _IO, _Conf, _View, _LCr=[]):
self.Conf = _Conf
self.View = _View
self.LCr = _LCr
self.pathQDSpy = fsu.getQDSpyPath()
self.ShManager = csh.ShaderManager(self.Conf, self.pathQDSpy)
self.ShManager = csh.ShaderManager(self.Conf)
self.useSound = glo.QDSpy_isUseSound
self.reset()

Expand All @@ -84,11 +82,10 @@ def __init__(self, _Stage, _IO, _Conf, _View, _LCr=[]):
if self.useSound:
Log.write("DEBUG", "Loading sounds ...")
self.SoundPlayer = SoundPlayer()
p = self.pathQDSpy +glo.QDSpy_pathSounds
self.SoundPlayer.add(Sounds.OK, p +glo.QDSpy_soundOk)
self.SoundPlayer.add(Sounds.ERROR, p +glo.QDSpy_soundError)
self.SoundPlayer.add(Sounds.STIM_START, p +glo.QDSpy_soundStimStart)
self.SoundPlayer.add(Sounds.STIM_END, p +glo.QDSpy_soundStimEnd)
self.SoundPlayer.add(Sounds.OK, os.path.join(glo.QDSpy_pathSounds, glo.QDSpy_soundOk))
self.SoundPlayer.add(Sounds.ERROR, os.path.join(glo.QDSpy_pathSounds, glo.QDSpy_soundError))
self.SoundPlayer.add(Sounds.STIM_START, os.path.join(glo.QDSpy_pathSounds, glo.QDSpy_soundStimStart))
self.SoundPlayer.add(Sounds.STIM_END, os.path.join(glo.QDSpy_pathSounds, glo.QDSpy_soundStimEnd))
Log.write("DEBUG", "... done")
else:
self.SoundPlayer = None
Expand Down
8 changes: 3 additions & 5 deletions QDSpy_core_shader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import os
import QDSpy_global as glo
import QDSpy_file_support as fsu
import Libraries.log_helper as _log
from Graphics.shader_opengl import Shader

Expand All @@ -37,24 +36,23 @@ class ShaderFileCmd:
# Shader manager class
# ---------------------------------------------------------------------
class ShaderManager:
def __init__(self, _Conf, _path: str):
def __init__(self, _Conf):
# Initializing
self.Conf = _Conf
self.ShFileList = []
self.ShDesc = []
self.ShTypes = []
self.ShVertCode = []
self.ShFragCode = []
pshader = fsu.repairPath(_path +self.Conf.pathShader)

# Make a list of the available shader files ...
f = []
for dirpath, dirnames, filenames in os.walk(pshader):
for dirpath, dirnames, filenames in os.walk(self.Conf.pathShader):
f.extend(filenames)
break
for fName in f:
if (os.path.splitext(fName)[1]).lower() == glo.QDSpy_shaderFileExt:
self.ShFileList.append(pshader + fName)
self.ShFileList.append(self.Conf.pathShader + fName)

# Parse each shader file ...
isInVert = False
Expand Down
8 changes: 4 additions & 4 deletions QDSpy_file_support.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ def getFNameNoExt(_fName):
def getStimCompileState(_fName: str) -> bool:
""" Check if pickle-file is current
"""
fPath = os.path.splitext(repairPath(_fName))[0]
dirPath = os.path.splitext(_fName)[0]
#print("getStimCompileState2", _fName, fPath)
try:
tStamp = os.path.getmtime(fPath + glo.QDSpy_stimFileExt)
tStamp = os.path.getmtime(os.path.join(dirPath, glo.QDSpy_stimFileExt))
tPy = datetime.fromtimestamp(tStamp)
tStamp = os.path.getmtime(fPath + glo.QDSpy_cPickleFileExt)
tStamp = os.path.getmtime(os.path.join(dirPath, glo.QDSpy_cPickleFileExt))
tPck = datetime.fromtimestamp(tStamp)
return tPck > tPy

Expand All @@ -69,7 +69,7 @@ def getStimCompileState(_fName: str) -> bool:
def getStimExists(_fName):
""" Check if stimulus file (.py) exists
"""
fPath = repairPath(_fName) + glo.QDSpy_stimFileExt
fPath = _fName + glo.QDSpy_stimFileExt
#print("getStimExists", fPath, os.path.isfile(fPath))
return os.path.isfile(fPath)

Expand Down
13 changes: 8 additions & 5 deletions QDSpy_global.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
2024-06-15 - Fix for breaking change in `configparser`; now using
`ConfigParser` instead of `RawConfigParser`
"""
import os.path

# ---------------------------------------------------------------------
__author__ = "[email protected]"

# fmt: off
# ---------------------------------------------------------------------
QDSpy_path = os.path.dirname(__file__)
QDSpy_versionStr = "QDSpy v0.92 beta"
QDSpy_copyrightStr = "(c) 2013-24 Thomas Euler"
QDSpy_appID = u"QDSpy3.v090beta.thomas_euler.eulerlab.de"
Expand All @@ -26,7 +29,7 @@
QDSpy_loop_sleep_s = 0.01

QDSpy_isUseSound = False
QDSpy_pathSounds = ".\\Sounds\\"
QDSpy_pathSounds = os.path.join(QDSpy_path, "Sounds") + os.path.sep
QDSpy_soundStimStart = "stim_start.mp3"
QDSpy_soundStimEnd = "stim_end.mp3"
QDSpy_soundError = "error.mp3"
Expand Down Expand Up @@ -72,7 +75,7 @@
QDSpy_cPickleFileExt = ".pickle"
QDSpy_fileVersionID = 8
QDSpy_stimFileExt = ".py"
QDSpy_pathStimuli = ".\\Stimuli\\"
QDSpy_pathStimuli = os.path.join(QDSpy_path, "Stimuli") + os.path.sep
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Very minor point] If you are switching to os.path.join everywhere, I think you can drop adding the os.path.sep at the end, as it will be added any time you join a path onto this path.

Also, if you are just starting on the task of reworking path management, I'd choose pathlib over os.path, as the former is easier to work with. Just my opinion though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are switching to os.path.join everywhere, I think you can drop adding the os.path.sep at the end, as it will be added any time you join a path onto this path.

I was not sure if at any points in the code that I missed we still use string concatenation (I searched for \\ in the code and adapted all those occurrences, but there might be some I missed that make the asusmption that every path in the global.py file ends with a path separator.

Also, if you are just starting on the task of reworking path management, I'd choose pathlib over os.path, as the former is easier to work with. Just my opinion though.

I only adjusted the code so far that my initial use case worked on Linux, which lead to the error in the PR description, and spend some effort to make the code consistent wrt to path concatenation. I would leave it at that, though. Also if you'd want to spend more time on it, feel free to directly adjust this PR to use pathlib.

Did you happen to use this branch? If so did it work, and did you use it on Windows or Linux?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear both,

sorry for replying so late.

Path management can be really annoying and I agree with the need of a better solution.
However, I am not too happy with the solution you suggested @thomasZen because I expect side effects on the Windows users. I haven't tested the changes yet, but instead suggest a different procedure:

(1) The reason I introduced QDSpy_file_support.py was to have all path/file related functions in one place, where one can take care of OS-specific differences. Therefore, I suggest to change the functions there instead and adding support functions if needed, instead on patching the code. E.g., instead of adding os.path.join() using a function in QDSpy_file_support.py, where we can implement the function either with pathlib or os.path. (see below) This way, we prevent mixing the libraries and stay flexible.

(2) I agree with @kevindoran in that pathlib is likely the better choice. I used it for a different project and had surprisingly free problems with paths on three different systems (Windows, Linux, and a proprietary DOS-like OS).

I can try to make a suggestion based on experimental and your changes @thomasZen and then you can test, o.k.?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that sounds good. I'm happy with testing anything on Linux.

Also feel free to delete this PR and branch once it's no longer needed.

QDSpy_autorunStimFileName = "__autorun"
QDSpy_autorunDefFileName = "__autorun_default_DO_NOT_DELETE"

Expand All @@ -87,10 +90,10 @@

QDSpy_vidAllowedVideoExts = [".avi"]

QDSpy_pathApplication = ".\\"
QDSpy_iniFileName = "QDSpy.ini"
QDSpy_pathApplication = QDSpy_path
QDSpy_iniFileName = os.path.join(QDSpy_path, "QDSpy.ini")

QDSpy_pathLogFiles = ".\\Logs\\"
QDSpy_pathLogFiles = os.path.join(QDSpy_path, "Logs") + os.path.sep
QDSpy_logFileExtension = ".log"
QDSpy_doLogTimeStamps = True

Expand Down
9 changes: 4 additions & 5 deletions QDSpy_stim.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,7 @@ def defShader(self, _shID, _shType):
shader type exists
"""
if self.ShManager is None:
_path = fsp.getQDSpyPath()
self.ShManager = csh.ShaderManager(self.Conf, _path)
self.ShManager = csh.ShaderManager(self.Conf)
if _shType not in self.ShManager.getShaderTypes():
self.LastErrC = StimErrC.invalidShaderType
raise StimException(StimErrC.invalidShaderType)
Expand Down Expand Up @@ -1381,13 +1380,13 @@ def load(self, _sFileName: str, _onlyInfo: bool = False):
if not (_onlyInfo):
_log.Log.write(" ", "Loading compiled stimulus...", True)

sPath = fsp.repairPath(_sFileName)
sPath = _sFileName
try:
with open(sPath + glo.QDSpy_cPickleFileExt, "rb") as stimFile:
with open(_sFileName + glo.QDSpy_cPickleFileExt, "rb") as stimFile:
'''
self.fileName = sFileName.replace("\\\\", "\\")
'''
self.fileName = sPath
self.fileName = sPath
#print("load", sPath)
stimPick = pickle.Unpickler(stimFile)
ID = stimPick.load()
Expand Down
18 changes: 3 additions & 15 deletions QDSpy_stim_movie.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,12 @@
__author__ = "[email protected]"

import os.path
import platform
import configparser
import QDSpy_stim as stm
import QDSpy_global as glo
import QDSpy_file_support as fsu
import Libraries.log_helper as _log
import Graphics.renderer_opengl as rdr

PLATFORM_WINDOWS = platform.system() == "Windows"

# ---------------------------------------------------------------------
# Movie object class
# ---------------------------------------------------------------------
Expand Down Expand Up @@ -159,17 +155,9 @@ def load(self, _fName, _testOnly=False):
tempStr = (os.path.splitext(os.path.basename(_fName)))[0]
self.fExtImg = os.path.splitext(_fName)[1].lower()
self.isTestOnly = _testOnly

if PLATFORM_WINDOWS:
tempDir = os.path.dirname(_fName)
if len(tempDir) > 0:
tempDir += "\\"
self.fNameDesc = tempDir + tempStr + glo.QDSpy_movDescFileExt
self.fNameImg = _fName
else:
tempDir = os.getcwd()
self.fNameDesc = fsu.repairPath(tempDir + tempStr) + glo.QDSpy_movDescFileExt
self.fNameImg = fsu.repairPath(tempDir + tempStr) + self.fExtImg
tempDir = os.path.dirname(_fName)
self.fNameDesc = os.path.join(tempDir, tempStr) + glo.QDSpy_movDescFileExt
self.fNameImg = _fName

if self.fExtImg in glo.QDSpy_movAllowedMovieExts:
return self.__loadMontage()
Expand Down
12 changes: 1 addition & 11 deletions QDSpy_stim_video.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import platform
import QDSpy_stim as stm
import QDSpy_global as glo
import QDSpy_file_support as fsu
import Libraries.log_helper as _log
import moviepy.editor as mpe
import Graphics.renderer_opengl as rdr
Expand Down Expand Up @@ -83,18 +82,9 @@ def load(self, _fName, _testOnly=False):
"""Reads a movie file (e.g. AVI); a description file is not needed
Returns an error of the QDSpy_stim.StimErrC class
"""
tempStr = (os.path.splitext(os.path.basename(_fName)))[0]
self.fExtVideo = os.path.splitext(_fName)[1].lower()
self.isTestOnly = _testOnly

if PLATFORM_WINDOWS:
tempDir = os.path.dirname(_fName)
if len(tempDir) > 0:
tempDir += "\\"
self.fNameVideo = _fName
else:
tempDir = os.getcwd()
self.fNameVideo = fsu.repairPath(tempDir + tempStr) + self.fExtVideo
self.fNameVideo = _fName

if self.fExtVideo in glo.QDSpy_vidAllowedVideoExts:
return self.__loadVideo()
Expand Down