-
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
Check pyiron installation #1131
Conversation
Current checks implemented: - executable bit is set for shell scripts - conda package is installed These can be executed using: ``` import pyiron_base pyiron_base.check_executables_status() ``` Which returns a pandas dataframe with all executables pyiron found.
Would be nice to have this also available via |
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.
This still needs a test. We can explicitly add some dummy scripts to the static directory. Whenever there is path based stuff, it's much safe to have it appear in the CI so we don't accidentally specify something in a windows non-compliant way.
|
||
path_lst = [] | ||
for res_path in resource_paths: | ||
path_lst += glob.glob(res_path + "/*/*/*.sh") |
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.
This feels very fragile. It looks to me like this requires all executables to be at exactly the third level of depth. This may be the case right now, but so we specify or document this anywhere? Do we anticipate other sh files appearing here, or is it safe to run an endswith command recursively inside the resource path instead?
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.
Right now it needs to be exactly at the third level because the expected structure is /$resources/$module_name/bin/$exe.sh
. The documentation for this is here, though it could be more explicit.
I have a little class that codifies this in tinybase which could be moved to base. Replacing this everywhere might be a bit more effort, so imo it's ok to do it in a separate PR.
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.
I'm ok with codefying it as long as we're sufficiently explicit. One option I see is to search for scripts at exactly three levels deep and also searching for them anywhere in the resource tree, then giving the user a warning if the lists aren't the same.
Actually, I think I really like that, and it should be straightforward to make a test setup in static to make sure both aspects work.
Similarly we should also be checking if non-supported codes have directories. I'm on mobile so I can't easily bail on this comment to check if that's already done, but this would be another nice thing to give a warning about.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Current checks implemented:
These can be executed using:
Which returns a pandas dataframe with all executables pyiron found.