pyiron / pyiron_workflow

Graph-and-node based workflows
BSD 3-Clause "New" or "Revised" License
12 stars 1 forks source link

Add a shortcut for removing file(s) by name #159

Closed liamhuber closed 10 months ago

liamhuber commented 10 months ago

Introduces the following convenience method to DirectoryObject:

    def remove(self, *files: str):
        for file in files:
            path = self.get_path(file)
            if path.is_file():
                path.unlink()
github-actions[bot] commented 10 months ago

Binder :point_left: Launch a binder notebook on branch _pyiron/pyiron_workflow/remove_filemethod

codacy-production[bot] commented 10 months ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
:white_check_mark: +0.03% (target: -1.00%) :white_check_mark: 100.00%
Coverage variation details | | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (35b6d77ef7b1d04a97d22c213d6331d009b09438) | 2572 | 2195 | 85.34% | | | Head commit (33fad04882fbe819e60724c7dc8fd8b6eb50d63b) | 2577 (+5) | 2200 (+5) | 85.37% (**+0.03%**) | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `
Diff coverage details | | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#159) | 5 | 5 | **100.00%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

samwaseda commented 10 months ago

Actually I don't really get the intention of this PR. I think by default people should delete the file from FileObject and not from the directory (except when all files should be erased)

liamhuber commented 10 months ago

Wouldn't that involve importing FoleObject and creating N of these based on the strings used in this shortcut and the DirectoryObject? This isn't horrid, I just wanted something even shorter. You can see the use in Node.__post__ over in the storage PR

github-actions[bot] commented 10 months ago

Pull Request Test Coverage Report for Build 7534218211


Totals Coverage Status
Change from base Build 7534135240: 0.02%
Covered Lines: 4676
Relevant Lines: 5179

💛 - Coveralls
liamhuber commented 10 months ago

@samwaseda if, on reflection, you think this is useful, feel free to merge it. However, I came around to your perspective that importing and instantiating a FileObject is also perfectly acceptable, and in my case (since I have only the one file I care about and already import DirectoryObject) not particularly more work. I've dropped it from the history of #160 and don't depend on it.

So if you still dislike it, just close the PR; I still don't hate it, but am quite indifferent now.

samwaseda commented 10 months ago

Still not really sure about this one, but I also don't feel strongly against it, so I merged it :D