TEASER icon indicating copy to clipboard operation
TEASER copied to clipboard

Bug in class DataClass when loading read-only files

Open marioaieie opened this issue 11 months ago • 1 comments

What is the problem?

  • The load methods in the DataClass class fail when loading a read-only file.
../teaser/project.py:120: in __init__
    self.data = self.instantiate_data_class()
../teaser/project.py:136: in instantiate_data_class
    return DataClass()
../teaser/data/dataclass.py:56: in __init__
    self.load_tb_binding()
../teaser/data/dataclass.py:88: in load_tb_binding
    with open(self.path_tb, "r+") as f:
E   PermissionError: [Errno 13] Permission denied: 'TEASER/teaser/data/input/inputdata/TypeBuildingElements.json'

Why do we want to solve it?

  • For example, to be able to run teaser on clusters where the python libraries might be installed in a read only filesystem

How do we want to solve it?

  • The mode of open should be set to "r" instead of "r+"

marioaieie avatar Feb 28 '24 09:02 marioaieie

I have a branch with a fix for this bug, however I couldn't create add a new test to verify the fix as it will require to change the permission of the files during a test run

marioaieie avatar Feb 28 '24 10:02 marioaieie

@DaJansenGit, can somebody take an action here and support Mario? He is a colleague of mine. ;-)

mlauster avatar May 26 '24 19:05 mlauster

Sorry for joining the conversation a bit late. Thanks, @marioaieie, for pointing out the issue.

We can change the argument to r instead r+ as those are indeed just input files, but I don't get what you mean by

I couldn't create add a new test to verify the fix as it will require to change the permission of the files during a test run

As far as I see it, there are no changes or new tests required. The functions load_tb_binding(), load_uc_binding() and load_mat_binding() either create a file if a file is already set for self.path_<tb,uc,mat> is set or the given file is loaded, but then read only.

If a file is provided, only read access is needed, as the information from the file will be stored in self.<...>_bind and used from their. The file is no further needed.

If no file is provided the functions shouldn't fail even if TEASER is installed in read-only, because they will be directed to the TEASEROutput folder in your user directory.

I created a branch and a merge request.

DaJansenGit avatar May 27 '24 15:05 DaJansenGit

Thanks for fixing this so quickly.

I was just suggesting to add a test to make sure the functions don't fail, but as you said, it's not necessary.

Will you close the issue or should I do it?

marioaieie avatar Jun 03 '24 06:06 marioaieie

I close it. Thanks for pointing out the issue.

DaJansenGit avatar Jun 03 '24 06:06 DaJansenGit