-
Notifications
You must be signed in to change notification settings - Fork 1
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
Tarball all needed templates from different folder #214
Conversation
Pull Request Test Coverage Report for Build 10934406788Details
💛 - Coveralls |
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.
Really nice solutionwith the lockable set! Looks good to me in general but I haven't tried it (yet). One small concern are the non-flat structures in the tar ball but I might be wrong here.
One question not entirely related to this PR: Shouldn't the toyfiles be included here as well? Since we no allow toydata mode "read" they should be tarred as well or am I missing something?
logger.warning( | ||
"The template path contains subdirectories. All templates files will be tarred." | ||
) | ||
|
||
def _tar_h5_files(self, directory, template_tarball="templates.tar.gz"): | ||
"""Tar all .h5 templates in the directory and its subdirectories into a tarball.""" |
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.
The docstring should be updated
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.
Yes, you are right.
for dirpath, dirnames, filenames in os.walk(self.template_path): | ||
for filename in filenames: | ||
logger.info(f"File: {filename} in {dirpath}") | ||
if self._contains_subdirectories(self.template_path): |
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.
_contains_subdirectories
is no longer used after this PR sop we might as well delete it or am I missing something?
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.
Yes, you are right.
Toydata is added to the file in this line: alea/alea/submitters/htcondor.py Line 593 in ba824ac
|
as long as their names are different
Asking all templates are in the same folder does not make sense, configuration like: https://github.com/XENONnT/nton/blob/fcccf25d583e1ee218724c4962d6fac9583c5191/nton/wimp/configs/sr0_sr1/wimp_statistical_model_ac_optimization.yaml#L409 and https://github.com/XENONnT/nton/blob/fcccf25d583e1ee218724c4962d6fac9583c5191/nton/wimp/configs/sr0_sr1/wimp_statistical_model_ac_optimization.yaml#L553 should be allowed.
LockableSet
and initialize a variableTEMPLATE_RECORDS
inalea/utils.py
TEMPLATE_RECORDS
inalea.utils._prefix_file_path
, soTEMPLATE_RECORDS
will record all needed templates absolute pathalea.submitters.htcondor.SubmitterHTCondor._validate_template_path
oralea.submitters.htcondor.SubmitterHTCondor._check_filename_unique
any morealea.submitters.htcondor.SubmitterHTCondor._tar_h5_files
, first copy all templates intotemplates_tarball_dir
then make tarball oftemplates_tarball_dir