Implements basic importer for Audacity project files #2
Conversation
…aup. Includes deserializing and loading expectations. Includes basic tests.
There was a problem hiding this comment.
So confirm for me: should there be one audacity project file per audio file?
And are those project files required to have the same name as the audio file?
If not, then we'll need to rethink the structure of this a bit. If so, see comments
Also I'm going to guess you didn't run the app? Unless I'm mistaken not enough of the new services were registered in the DI system.
Otherwise, excellent 😁
| { | ||
| if (context.Include.Filter is not null) | ||
| { | ||
| throw new Exception("The Audacity importer does not currently support include filters"); |
There was a problem hiding this comment.
There's nothing useful to filter on in avianz data files, but you could filter the different audacity tracks by name?
| Name = name, | ||
| }; | ||
| })) | ||
| .ToList() ?? new List<IExpectation> {new NoEvents()}; |
There was a problem hiding this comment.
I'd prefer to avoid the allocation of the list here
|
|
||
| logger.LogTrace("Data file converted to expectations: {@expectations}", expectations); | ||
|
|
||
| yield return new TestCase {SourceInfo = dataFile?.SourceInfo, Expect = expectations.ToArray(),}; |
There was a problem hiding this comment.
dataFile should never be null right? If it is, then we should fall back to providing some source info (like the path) so in error cases at least we can print a message that point to the right source file.
| test_suites: | ||
| host_suite: | ||
| tests: | ||
| - file: bird*.wav |
There was a problem hiding this comment.
this line of the config should be redundant
in an abstract sense: the main egret config is importing other tests (e.g. from another egret file, or an aup file acting like an egret file) and those tests should be referencing audio files.
tests/Egret.Tests/Serialization/Audacity/AudacityImporterTests.cs
Outdated
Show resolved
Hide resolved
tests/Egret.Tests/Serialization/Audacity/AudacityImporterTests.cs
Outdated
Show resolved
Hide resolved
|
|
||
| var tests = testsCases[0].Expect; | ||
|
|
||
| tests[0].Should().BeOfType<BoundedExpectation>(); |
There was a problem hiding this comment.
The generated test should have it's File property set, test that
|
|
||
| logger.LogTrace("Data file converted to expectations: {@expectations}", expectations); | ||
|
|
||
| yield return new TestCase {SourceInfo = dataFile?.SourceInfo, Expect = expectations.ToArray(),}; |
There was a problem hiding this comment.
The File property is not set - it must be set. The assumption here is that the importer knows which file the metadata is referring too.
Note to self: should probably make TestCase throw if both File and Url are null or empty
There was a problem hiding this comment.
As in
…entation. Registered relevant classes as services. Improved tests. Made XmlSerializer work as a service.
There was a problem hiding this comment.
Since there seems to be no strong link between audacity projects and audio files we're going to enforce a convention.
The file /123/abc.wav is related to the file /123/abc.aup or /123/abc.aup3 IFF the directory and basename are equivalent (i.e. they are siblings). When outputting results a source of /123/abc.wav will produce /output/abc.aup.
It appears the formatter already does this.
I think you should remove the entity framework package. The audacity3 serializer even only uses Microsoft.Data.Sqlite which AFAIK is its own package.
Probably only need one audacity importer (it can switch which serializer it uses based on file extension). As far as I can tell, the two importers do the same thing?
| // TODO: audacity projects can store audio files with the project file, | ||
| // but the audio files are split into multiple smaller files. | ||
| // File = Path.GetRelativePath(Path.GetDirectoryName(path), testFile) |
There was a problem hiding this comment.
See summary comments - we should find a source file with a similar name next to the project file, or fail
| var testsCases = includeTests.Tests; | ||
| testsCases.ToList().Should().HaveCount(1); | ||
|
|
||
| var expectations = testsCases[0].Expect; |
There was a problem hiding this comment.
should assert test's source is set correctly
Addressed feedback - audio file convention, single importer, moved fixture data. Partial implementation of audacity result formatter.
|
I'm now able to run egret with audacity project files as input, and get audacity files as output! I'm not sure about how to write the results to the Audacity Project Tracks and labels - there's a few TODOs in there that will need your help @atruskie. I've made the second round of changes to address your feedback. Ready for another review. |
File extension
.aup.Includes deserializing and loading expectations.
Includes basic tests.
Initial step for #1.