Skip to content

Conversation

troycomi
Copy link
Collaborator

Check if you need to strip quotations from filenames for categoricals and files
Detect if categorical has file (hd5) separators, warn user and escape to store as categorical

@troycomi troycomi changed the title WIP: add md5 and basename file parsers Add md5 and basename file parsers Apr 25, 2025
@troycomi
Copy link
Collaborator Author

Closes #8

I updated the config code to use default resources from the toml instead of the hard coded values. The configuration toml is now documented in the readme and when the user omits the toml option they get an error telling them it's required and to check the readme for an example.

Need to still work on categorical inputs that are paths (handling, warning, etc).
I think we could improve the toml handling more; if the option isn't set look for a standard file, like slurmise.toml or pyproject.toml for that information.

This is good to merge now though, can add the rest later.

@troycomi troycomi requested a review from iparask April 25, 2025 12:47
Copy link
Collaborator

@iparask iparask left a comment

Choose a reason for hiding this comment

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

I think it looks good. I just left a few comments in the README

# can be overwritten by subsequent jobs
# if not set, will use 1000 for default memory and 60 for default runtime
default_mem = 2000
default_time = 70
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default_time = 70
default_time = 60

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, but I had this as 70 to show it could be different than the defaults when nothing is provided (1000 and 60).

README.md Outdated
# /^>/ {next}
# {seq = seq + length($0)}
# END {if (seq) print seq}
'''
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you forgot the opening of these.

Co-authored-by: Ioannis Paraskevakos <[email protected]>
@troycomi troycomi merged commit e50599c into main May 1, 2025
3 checks passed
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.

2 participants