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.

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

Replace os by pathlib to avoid OS related errors and improve code maintenance #427

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

BishopWolf
Copy link

This PR fixes fresh installations errors when parent folders are not created by default giving

FileNotFoundError: [Errno 2] No such file or directory: '...

These errors were triggered by some tests and are all solved by this PR. The change ensures the folders AND all their parents are created.

@BishopWolf BishopWolf changed the title fix errors on bad os.mkdir calls (parents non exiting) fix errors on bad os.mkdir calls (parents non existing) Jul 15, 2024
@dsarrut
Copy link
Contributor

dsarrut commented Jul 16, 2024

Excellent, thanks! However, I got: AttributeError: module 'os' has no attribute 'mkdirs'
maybe makedirs ?

@BishopWolf
Copy link
Author

BishopWolf commented Jul 17, 2024

Excellent, thanks! However, I got: AttributeError: module 'os' has no attribute 'mkdirs' maybe makedirs ?

I have changed some os calls by pathlib. This is very easy to maintain as all OS related specificities are automatically handled.

@BishopWolf BishopWolf changed the title fix errors on bad os.mkdir calls (parents non existing) replace os by pathlib to avoid OS related errors and improve code maintenance Jul 17, 2024
@BishopWolf
Copy link
Author

@BishopWolf BishopWolf changed the title replace os by pathlib to avoid OS related errors and improve code maintenance [Draft] replace os by pathlib to avoid OS related errors and improve code maintenance Jul 17, 2024
@BishopWolf
Copy link
Author

I am solving all errors caught by the tests

@BishopWolf
Copy link
Author

BishopWolf commented Jul 18, 2024

There are still places expecting string paths instead of path objects. As a rule, you can't pass strings to methods expecting paths, as it's very difficult to maintain. This is why pathlib is preferred.
Example:

path1 = "/some/path"
path2 = "/some/path/"

# adding a file pathlib way
file1 = Path(path1) / "myfile"
file2 = Path(path2) / "myfile"

assert(str(file1) == str(file2)) # returns true

# os way
file1 = os.path.join(path1, "myfile")
file2 = os.path.join(path2, "myfile")

assert(file1== file2) # returns false
# file1 is "/some/path/myfile" in Nix and "/some/path\myfile" in Windows
# file2 is "/some/path//myfile" in Nix and "/some/path/\myfile" in Windows

@BishopWolf BishopWolf marked this pull request as draft July 18, 2024 12:16
@BishopWolf
Copy link
Author

I see some merge conflicts with the last pull in wget. @tbaudier may you please review?

@BishopWolf
Copy link
Author

The only error remaining in test040_gan_phsp_pet_gan.py

File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/opengate/actors/arfactors.py", line 243, in apply_with_lock
    w = self.garf.nn_predict(self.model, self.nn["model_data"], ax)
        ^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'garf' has no attribute 'nn_predict'

@BishopWolf BishopWolf changed the title [Draft] replace os by pathlib to avoid OS related errors and improve code maintenance Replace os by pathlib to avoid OS related errors and improve code maintenance Jul 24, 2024
@tbaudier tbaudier force-pushed the fix-mkdirs branch 4 times, most recently from 3a174b7 to 84f1aff Compare July 24, 2024 15:28
@BishopWolf BishopWolf marked this pull request as ready for review July 25, 2024 07:36
@BishopWolf
Copy link
Author

@tbaudier : all tests are passing now

@BishopWolf
Copy link
Author

@tbaudier : my tests didn't show any errors, see https://github.com/BishopWolf/opengate/actions/runs/10093731109/job/27911347164

Something is weird

@tbaudier
Copy link
Contributor

@tbaudier : my tests didn't show any errors, see https://github.com/BishopWolf/opengate/actions/runs/10093731109/job/27911347164

Something is weird

I will look at your PR, I think it's due to the rebasing of the branch. Do not modify it for the moment and I will try to do the necessary.
Thomas

@BishopWolf
Copy link
Author

This error is because filename shall be Path:

 File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/opengate/engines.py", line 612, in stop_simulation
    actor.EndSimulationAction()
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/opengate/actors/miscactors.py", line 126, in EndSimulationAction
    self.write(self.user_info.output)
  File "/opt/hostedtoolcache/Python/3.11.9/x64/lib/python3.11/site-packages/opengate/actors/miscactors.py", line 146, in write
    with filename.open("w") as f:
         ^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'open'

But it's being called by
image

and user_info.output is just a filename string without path

replace os by pathlib in all data setup utilities

replace ".." by parent as pathlib standard

handle bug in get test path

fix errors shown by tests
Select onlyfiles avoiding glob because it select *.pyc files wich do not exist
parent / opengate / tests / data does not exists and it will be created during the downloading
ct_4mm.raw cannot be find in data_location.iterdir()

[pre-commit.ci] Automatic python and c++ formatting
@tbaudier tbaudier merged commit b176701 into OpenGATE:master Jul 29, 2024
24 of 27 checks passed
@BishopWolf BishopWolf deleted the fix-mkdirs branch July 29, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants