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

Add FileOpener module #116

Closed
wants to merge 2 commits into from
Closed

Add FileOpener module #116

wants to merge 2 commits into from

Conversation

tcmoore3
Copy link
Member

Description, motivation, and context

This PR adds a module that enables to open files with the system file viewer as determined by the OS.
One issue I run into while exploring a project in the dashboard is when I see something interesting in the data displayed in the dashboard and want to look into that job in more detail by opening the trajectory in OVITO. When this happens, I have to go my terminal type open workspace/<first few characters of job id><tab><tab><tab>/trajectory.gsd, which can interrupt my workflow. When browsing through tens to hundreds of jobs, this time can add up to a signfiicant amount of time (not to mention the added mental energy barrier). This PR makes it possible to run this command directly from the dashboard at the click of a button.

The current implementation takes two arguments: the "open command" and the filename. The open command can vary by operating system (on macOS, just using open works; I'm not sure what this should be on Windows or Linux). This implementation also currently just uses os.system since I found it simple enough for my use case. Current limitations are:

  1. Only a single FileOpener module can be registered (for the same reason as the Notes module as documented in Only 1 Notes instance appears on dashboard #74)
  2. This probably won't work (at least not in any meaningful way) if running the dashboard remotely.

Types of Changes

  • Documentation update
  • Bug fix
  • New feature
  • Breaking change1

1The change breaks (or has the potential to break) existing functionality.

Checklist:

If necessary:

  • I have updated the API documentation as part of the package doc-strings.
  • I have updated the changelog.

@tcmoore3 tcmoore3 requested review from bdice and a team as code owners February 18, 2022 19:09
@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #116 (7d978de) into master (a586efc) will decrease coverage by 0.25%.
The diff coverage is 57.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage   63.07%   62.82%   -0.26%     
==========================================
  Files          18       19       +1     
  Lines         669      702      +33     
==========================================
+ Hits          422      441      +19     
- Misses        247      261      +14     
Impacted Files Coverage Δ
signac_dashboard/modules/file_opener.py 56.25% <56.25%> (ø)
signac_dashboard/modules/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a586efc...7d978de. Read the comment docs.

return "File not found."
full_filename = job.fn(self.filename)
cmd = f"{self.open_cmd} {full_filename}"
os.system(cmd)
Copy link
Member

@bdice bdice Feb 18, 2022

Choose a reason for hiding this comment

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

I might recommend using the default handler for a given file type? It depends a bit on exactly what you want (the equivalent of a double-click in Finder/Explorer vs. an arbitrary terminal command?) but this is one way to achieve that: https://stackoverflow.com/a/435669

Copy link
Member Author

Choose a reason for hiding this comment

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

This will use the default handler for a given file type, at least on mac. I indeed do want the equivalent of a double click on in Finder/Explorer, and it looks like using the subprocess module is better than os.system().

from signac_dashboard.module import Module


class FileOpener(Module):
Copy link
Member

Choose a reason for hiding this comment

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

We might be able to make an equivalent of the FileList module that opens instead of downloading the file. Or both, with buttons for either open/download action. Perhaps with a regex filter of filenames to display?

@bdice
Copy link
Member

bdice commented Feb 28, 2022

@tcmoore3 Instead of adding a module, we could change the FileList module to accept a glob (for filtration), and add a flag FileList(..., allow_open=False). If True, this would include a button to open the file, next to each download link. Just offering an alternate design idea to get the most useful features for the fewest number of separate modules.

This is the button icon I would use: image
https://fontawesome.com/icons/arrow-up-right-from-square?s=solid

What do you think about that idea?

@b-butler
Copy link
Member

@tcmoore3 do you still have interest in this feature/PR? If so do you have time to complete this? Also, what are your thoughts on Bradley's suggestion?

@tcmoore3
Copy link
Member Author

@tcmoore3 Instead of adding a module, we could change the FileList module to accept a glob (for filtration), and add a flag FileList(..., allow_open=False). If True, this would include a button to open the file, next to each download link. Just offering an alternate design idea to get the most useful features for the fewest number of separate modules.

This is the button icon I would use: image https://fontawesome.com/icons/arrow-up-right-from-square?s=solid

What do you think about that idea?

I love that idea. This would be really useful for hybrid remote/local work.

@tcmoore3
Copy link
Member Author

@b-butler My interest has dropped quite a bit (although not to zero!). I don't really have the bandwidth to complete this at the moment, so I'm going to close this and reopen in the future if I come across a stronger need for it.

@tcmoore3 tcmoore3 closed this Apr 20, 2022
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.

3 participants