New loaders #47
Merged
New loaders #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…<path>); replaces ms3.resolve_dir() with utils.resolve_path();
…tly implemented derive from; this happens in preparation for the Loader(PipelineStep) and others in the future which do not operate on features
…eton for Music21Loader
…ucture table; datatypes not cleaned; values such as time signatures not propagated; many redundant columns by using parse_m21_object() a lot; that also slows the parsing down, profiler shows that the use of getattr() on m21 objects takes a lot of time)
…Ds currently a fallback to (folder, fname); switched order of __init__ arguments, making package_name mandatory, but not source (can be added later, or is not necessary when specifying a list of paths, PathFactory, etc...)
…ough the user of Loader.iter_package_descriptors()
| return tuple(ext if ext[0] == "." else f".{ext}" for ext in extensions) | ||
|
|
||
|
|
||
| class PathFactory(Iterable[str]): |
There was a problem hiding this comment.
There are quite too many paramaters, and a builder here would be nice. It would allow to reuse some "base" scanner, and to specialize them quite easily instead of having to redefine the whole thing everytime.
|
I can't say I really looked through the 2000+ lines of changes in any depth, since I already didn't really know the inner workings, but I left some comments, probably on something you weren't asking about though :) Namely, the naming of FeatureStep I find confusing. What is a FeatureStep, and what are its sibling classes? What other types of PipelineSteps are there, and it seems counter-intuitive that every Analyzer and Grouper would be a FeatureStep. |
…rks with the revamped DiMCAT
…l with localkey_mode
…_plots() to dimcat.plotting
…(), apply_slice_intervals(), relevant for slicing
…gument, which is intepreted as dtype
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pathsproperty (4e7c845) and allowsourceargument to be either a directory or an iterable of directories or an iterable/factory of filepaths -- adapt the Schema() and test serialization of loaders!callable(path) => (corpus, piece)path/filename => (corpus, piece)Welcoming your feedback and suggestions for a loader API, @huguesdevimeux and @apmcleod; I hope the demos in
tests/test_loaders.pygive a good enough idea for now. Documentation is yet to follow.I would also be thankful for some input concerning #44 (see above) which, ideally, I would like to see addressed with this PR, too.
JFYI, this version currently required the bleeding-edge
ms3@schema(specified in requirements) to generate JSON resource descriptors, so pleasepip install -e .[dev]to run the code.