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

Convert os.path to pathlib based Paths #646

Merged
merged 3 commits into from
Feb 5, 2025

Conversation

oerc0122
Copy link
Collaborator

Description of work

  • Replace os.path methods with pathlib.Paths
  • Rework MDANSE.Core.Platform to work with Paths rather than strs

To test
Nothing should change, standard tests.

@oerc0122 oerc0122 added the styling Any issue related to styling label Jan 28, 2025
@oerc0122 oerc0122 self-assigned this Jan 28, 2025
@@ -47,7 +46,7 @@ def create_structured_project(self):

class CurrentSession:
def __init__(self, fname=None):
self.settings_dir = os.path.join(expanduser("~"), ".MDANSE")
self.settings_dir = Path("~").expanduser() / ".MDANSE"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be noted this conflicts with the MDANSE.Core.Platform definition.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I can tell, CurrentSession.py is not referred to by any other file, since in the end the 'session' objects were implemented in the GUI instead. You're welcome to solve the conflict by deleting CurrentSession.py

@oerc0122 oerc0122 force-pushed the remove-os-path branch 17 times, most recently from b5fade0 to 6379fbf Compare January 30, 2025 21:40
@MBartkowiakSTFC
Copy link
Collaborator

At the moment, manual testing shows problems with saving the input parameters in the output files:

  File "MDANSE/Framework/Jobs/IJob.py", line 419, in run
    raise JobError(self, tb)
MDANSE.Framework.Jobs.IJob.JobError: Traceback (most recent call last):
  File "MDANSE/Framework/Jobs/IJob.py", line 412, in run
    self.finalize()
  File "MDANSE/Framework/Jobs/AngularCorrelation.py", line 213, in finalize
    self._outputData.write(
  File "MDANSE/Framework/OutputVariables/IOutputVariable.py", line 42, in write
    temp_format.write(basename, self, header, inputs)
  File "MDANSE/Framework/Formats/MDAFormat.py", line 64, in write
    HDFFormat.write(filename, data, header, run_instance, extension)
  File "MDANSE/Framework/Formats/HDFFormat.py", line 97, in write
    inputs = run_instance.output_configuration()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "MDANSE/Framework/Configurable.py", line 209, in output_configuration
    result[name] = conf.to_json()
                   ^^^^^^^^^^^^^^
  File "MDANSE/Framework/Configurators/IConfigurator.py", line 263, in to_json
    return self._encoder.encode(self._original_input)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[email protected]/3.11.9_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[email protected]/3.11.9_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "[email protected]/3.11.9_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type PosixPath is not JSON serializable

@oerc0122 oerc0122 force-pushed the remove-os-path branch 2 times, most recently from 17cc4a0 to bb87ec0 Compare February 5, 2025 10:47
Copy link
Collaborator

@MBartkowiakSTFC MBartkowiakSTFC left a comment

Choose a reason for hiding this comment

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

I did not find any other problems caused by the changes. It seems to work fine now. I agree that the specific test for encoding all the inputs to JSON can be added in a separate PR.

@oerc0122
Copy link
Collaborator Author

oerc0122 commented Feb 5, 2025

I did not find any other problems caused by the changes. It seems to work fine now. I agree that the specific test for encoding all the inputs to JSON can be added in a separate PR.

c.f. #658

@oerc0122 oerc0122 merged commit 3f1535c into ISISNeutronMuon:protos Feb 5, 2025
26 checks passed
@oerc0122 oerc0122 deleted the remove-os-path branch February 5, 2025 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
styling Any issue related to styling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants