Add the filelist interface
Fixes #52 (closed)
Merge request reports
Activity
Added 1 commit:
- e8849568 - Add another method for the filelistbiodatabase class
Mentioned in issue #52 (closed)
22 23 def _make_bio(self, files): 23 24 return [BioFile(client_id=f.client_id, path=f.path, file_id=f.id) for f in files] 24 25 25 def probe_file_sets(self, model_id=None, group='dev'): 26 def object_sets(self, groups='dev', protocol=None, purposes=None, model_ids=None): The object_sets function should not have a purpose, as
FileSet
's are only used for probing and, hence, the purpose is always"probe"
. All of the low-level databases that provideFileSet
's respect this. Theatnt
database does not provideFileSet
's by default. Only to make the test work with this database, here I need to add thepurposes="probe"
.This part is not related to filelist databases. I added it here so that we can close !52 (closed) altogether. Please fix this part as you see fit by pushing here.
- Resolved by Amir MOHAMMADI
Thanks @amohammadi
I don't think that we need to provide the
driver.py
and the according entry in thesetup.py
. The driver is useful only to list the contents of a proper database. As theBioFileListDatabase
is only an interface without real content, there is IMHO no need to register this database as a real database.I thought this
driver.py
will be used! apparently it's not used: https://gitlab.idiap.ch/bob/bob.db.voxforge/blob/ffilelist/bob/db/voxforge/driver.py Where was this functionality used before then?- bob/bio/base/database/filelist/query.py 0 → 100644
81 Specify a custom filename for the Z-norm scores filelists (default is 'for_znorm.lst') 82 83 use_dense_probe_file_list : bool or None 84 Specify which list to use among 'probes_filename' (dense) or 'scores_filename'. 85 If ``None`` it is tried to be estimated based on the given parameters. 86 87 keep_read_lists_in_memory : bool 88 If set to true, the lists are read only once and stored in memory 89 """ 90 91 def __init__( 92 self, 93 base_dir, 94 name, 95 protocol=None, 96 biofilecls=BioFile, Oh, I see what you are doing here. Just a short note: could you have parameter names that are more expressive here? I know that most of the parameters were that short already before, but the
base_dir
should rather be renamed tofilelists_directory
(or something similar) in order to avoid confusion with theoriginal_directory
.Also, I would like to have the
biofilecls
to be spelled out asbio_file_class
, anddev_subdir
todev_sub_directory
andeval_subdir
toeval_sub_directory
. We are breaking the API of these databases anyways. But I would let other people comment on this @andre.anjos @tiago.pereira@mguenther I am very busy as I have my candidacy exam approaching but I put this together here anyway because it seemed like it was easier to do it myself than to explain it. So if you want to remove the driver, go ahead. If you want to change the variable names, go ahead. I think your comments are valid.
@amohammadi Since you are busy, you did not need to do these changes today, before we all even agreed on them. It was too rushed. We are moving this functionality between different packages for the 4th time in the last few months. We had generic database classes in
bob.bio.db
, then we moved them tobob.bio.base
andbob.db.bio_filelist
, then again I was told to move everything back tobob.bio.db
, now it's again inbob.bio.base
. It's very erratic and wastes a lot of time.
I don't really know, where and if the
driver.py
has been used before. Maybe @andre.anjos knows that.@pkorshunov this is not rushed! As you have said we did this code moving several times but each time it did not make sense. First Tiago did it and then you. The problem was you did not read and understand the whole conversation and Tiago did not test his. This is something that comes out of our conversations by carefully taking people's comments into consideration and it is well tested in bob.bio.base, bob.bio.spear, and bob.db.voxforge.
Also, your efforts did not go to waste. This is as I said a combination of yours, Manuel's, and Tiago's efforts. If you are still not happy with this, let us know and someone (including you) can fix it.
mentioned in commit 2e7ac534