-
Notifications
You must be signed in to change notification settings - Fork 5
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 a warning for safer reading of FASTA files using IndexedFastaCustom
+ bumped GitHub Actions python version to 3.9
#122
Conversation
IndexedFastaCustom
IndexedFastaCustom
+ bumped GitHub Actions python version to 3.9
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.
Thanks Sagi!
Few comments inline
fasta_file_creation_time = os.path.getctime(self.filename) | ||
index_file_creation_time = os.path.getctime(self.index_filename) | ||
|
||
if fasta_file_creation_time > index_file_creation_time + 42: |
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.
Why the extra 42 seconds?
@@ -98,6 +99,16 @@ def __init__( | |||
print(f"index file already found: {index_filename}") | |||
already_found = True | |||
|
|||
if already_found: | |||
# Fasta file should be older than it's corresponding index file | |||
fasta_file_creation_time = os.path.getctime(self.filename) |
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.
maybe mtime is safer?
index_file_creation_time = os.path.getctime(self.index_filename) | ||
|
||
if fasta_file_creation_time > index_file_creation_time + 42: | ||
warnings.warn( |
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.
Maybe raise an error and suggest deleting the index file to recreate it?
@SagiPolaczek is it something you want to merge? |
Issue
A case that the code didn't cover yet:
IndexedFastaCustom
(.hdf5
files were created)IndexedFastaCustom
withforce_recreate_index=False
.Now the offset values are not matching the
.fasta
files content and the user didn't get any notice for that.Suggested (relaxed) fix
.fasta
files is newer than the.hdf5
(index) file by a margin bigger than 42 (arbitrary number less than a minute)