-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add output files class #1311
Add output files class #1311
Conversation
# Conflicts: # .ci_support/environment.yml # pyiron_base/jobs/job/util.py # setup.py
Also use a sub test for every line test
job[] is overloaded to perform many functions at once: 1. access to files 2. access to HDF stored data 3. access to child jobs This change adds a new attribute `.files` to `JobCore` to take over function 1 and deprecates methods on the job itself that are connected to this: `list_files` and `tail`.
@pmrv I merged your previous branch into this one and modified it to fulfil your requirements (based on the tests) as well as mine. From my perspective the pull request is now ready to be reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some small nits, but otherwise lgtm.
raise AttributeError(item) from None | ||
|
||
|
||
class File(str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class File(str): | |
class File: | |
__slots__ = ("_path",) | |
def __init__(self, path: str): | |
""" | |
Wrap a file. | |
Args: | |
path (str): path to file | |
""" | |
self._path = path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We not actually using any string functionality here and I'd rather not let people accidentally perform string operations on this object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why I would like to keep the string part in, is that then I can use os.path.join()
on the object, this is what I need to access the path.
working_directory=os.path.dirname(self), | ||
file_name=os.path.basename(self), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
working_directory=os.path.dirname(self), | |
file_name=os.path.basename(self), | |
working_directory=os.path.dirname(self._path), | |
file_name=os.path.basename(self._path), |
See above.
Co-authored-by: Marvin Poul <[email protected]>
Co-authored-by: Marvin Poul <[email protected]>
Co-authored-by: Marvin Poul <[email protected]>
In comparison to the previous work in #969 this pull request only lists the file paths for the files in the working directory.
Alternatively, we could also consider combining both pull requests.