Incorporated pathlib.Path compatibility #99
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In this commit I have added input (and explicit output) compatibility between pymapdl-reader and the pathlib.Path class. All file path-input arguments should now be accepted as either strings or pathlib.Path objects. Additionally there is no breaing change regarding the filename properties. These will continue to return strings. Wherever possible I have made it so that internally we store paths as pathlib.Path objects and they are returned as strings through properties. Setters additionally will accept strings or pathlib.Path objects. Unfortunately i have not been able to get the test suite working on my local machine and so have not been able to write tests to go with these changes. I would appreciate input from others regarding how to get the tests running and if my changes do break any existing tests. I do not believe so but I am very limited in how I can check.
Let's open a separate PR to decouple the testing requirements of |
Changed the suffix comparison to include the leading '.' because pathlib.Path.suffix does include it. Made sure that AnsysFile converts any potential paths to string before they are passed as arguments. Additionally split thermal test up until multiple tests within a class to make it easier to read. All existing tests pass with pathlib changes.
Added additional tests to check the input and output is correct for the 4 classes that use pathlib.Path now.
|
Ready for review! |
akaszynski
reviewed
Jan 28, 2022
Comment on lines
+142
to
+144
| def filename(self) -> str: | ||
| """String form of the filename. Accepts ``pathlib.Path`` and string objects when set.""" | ||
| return str(self._filename) |
There was a problem hiding this comment.
Agree with this for backwards compatibility.
akaszynski
approved these changes
Jan 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR I have added input (and explicit output) compatibility
between pymapdl-reader and the pathlib.Path class. All file path-input
arguments should now be accepted as either strings or pathlib.Path
objects. Additionally there is no breaking change regarding the filename
properties. These will continue to return strings. Wherever possible I
have made it so that internally we store paths as pathlib.Path objects
and they are returned as strings through properties. Setters
additionally will accept strings or pathlib.Path objects.
Users can access the pathlib.Path objects via new
pathlib_filenameproperties.I could not think of a better name, but suggestions are welcome.
Unfortunately i have not been able to get the test suite working on mylocal machine and so have not been able to write tests to go with these
changes. I would appreciate input from others regarding how to get the
tests running and if my changes do break any existing tests. I do not
believe so but I am very limited in how I can check.
With the latest commits and #103 I have managed to get my development version
working and confirmed the changes work as well as adding tests for them.
This PR is ready for review and merging.
(closes #98)