Restructuring Spotdl #812
Restructuring Spotdl #812
Conversation
Flesihing out the ideas behing restructuring/recoding of spot-dl
The basic Ideas behind the restructuring attempt have been put down and will hopefully be updated as and when required. Now we can get straight to the code buisness.
Starting to work of the restructuring proper. You won't find any new code here but then a little design before coding can go a long way. I do my design via markdown, you should find a lot of that here. 😁
Some messups with the last commit and me trying to revert to a previous commit. Damn it should have been named 'Problems & Solutions III' well, here is the fix. The interface definitions have been finished. If I just figure out the code class dubbed soulOfSpotDl and the 'tools' proposed I can get down to code. Cheerio. 😁
Fixed up some inconspicious typos, ran some hacky tests, minor changes to interface definitons and some (in a way) working code.
Fixed some typo's, updated working docs, did some tests on logging in ./Hacks and got a working and configured heirarchal logger set up.
Nothing fancy here, just some typo fixes, guideline updates, design notes and recycled code.
Restructured Temp floder to resemble a python package. Minor name changes, a few edits, lots of questions and untested code. I might even have a hacky implementaion of the Metadata Search Interface.
Not much code this time around, was (re)figuring out the interfaces/object ideas, still have to change the docs to mirror the changes in code.
added the gener look up. did the doc strings...
Updated logging guidelines for more clarity of the resulting logs, updated the logging messages interspersed throughout the code to match the new guidelines, updated workingDocs. Chucked in a package diagram for good measure, it forces your to take stalk and stop roming around in circles. Put up the new stuff since 'Problems & Solutions I' under README updates.
Got in fresh embedding code that should work in theory, tests not yet done. Slight changes to the package diag. Some more minor folder renamings and stuff. One step closer to completion (4 more steps to go, assuming someone else will write the search provider which I'm hoping will be YTmusic) Thinking of ways to highlight the not so great function input variable 'v2_version=' used in mutagen, it tells you absolutely nothing about what the variable does.
Some typos and a lott of code... ༼ つ ◕_◕ ༽つ (Yayy!!!)
Was adding fresh functionality to spotifyHelpers.py and added a few more spotify-api responses to the REFS folder for reference
So, my ideas is to do the downloads and conversions in parrlell to speeden up things. Threading is not prrlell processing - multiprocess is. Fiddled around with multiprocessing. Tried calculating the SHA512 checksum for 33,326 files. single process ~ 25mins, 16 process (since I use a 8core Hyper-V processor) ~ just 3 mins. Far better than I expected.
Looking into various lobraries that can be used to download audio from YouTube and also into the speedy format conversion issue
|
The structure looks good to me overall. I have mostly some style concerns at this point. I see you're using camelCase for naming your classes, functions/methods and variables. AFAIK this isn't the preferred way in python. The style guide indicates to use TitleCase for classes, lower_case for method/function and variable names. |
|
Also, let's not inherit from |
|
Ok, cool, I didn't know that about the One other thing of note about naming, the aim of a naming convention is to make the code more easy to read, and as a result easier to contribute to. Having potential contributors pause to think of naming styles (considering that PEP defines one of variables, function, classes, modules...) will just end up being a drag. Neither is spotdl so big (unlike the std-lib for which PEP was created) that it needs naming styles that indicate what is a function, what is a class and what is a variable. I looked through the PEP naming conventions (at stackoverflow), they also note that 'internal consistency matters most'. If I think about it now, underscores do add readability, do you think we have to change it? If yes, are we taking about underscore based naming or of the whole PEP8 convention. |
|
I'd say let's just stick with the whole of PEP8 convention. I personally do not conform to a few things either - such as the character count per line, but overall I think it's a nice base to conform to and it does help with readability at least for me (or perhaps I've grown used to it). spotdl is indeed not a really big code base but it's still big enough to get lost (the reason we're doing this refactor). The greater community also mostly comes from same common ground that is PEP8, and this will also make it easier to make use of code check tools such as flake8 which could be helpful to make sure new contributors adhere to the same code style. So, I don't see a good reason to not follow PEP8, at least for the naming convention. On a side note, let's also avoid spaces around "=" in kwargs. |
Copied from the code I wrote for the original library. Includes "test" code.
Also updated objects.md with additional metadata suggestions.
Also updated objects.md with additional metadata suggestions.
Also includes (commented out) code to get metadata.
…potify-downloader into reStructure/reCode
YouTube Music's search response is a sprawling, over-nested JS Object. Code to filter out unnecessary data from those musked responses capable of handling all of the common response structures.
Wrote up a partial YouTube Music Based search Provider based off @roketinventors original code. I'm sure his version will be better
|
“The primary goal of any programmer is managing complexity” ABC’s were actually implemented in this iteration as I thought ABC’s would help, they were subsequently removed as it didn’t help as much as expected. I do accept that the original authors found a necessity for ABC’s, just because it was there doesn’t mean it should continue to be there. Also, ABC’s were never used in production code. Most of what I’ve done with help from @rocketinventor with @ritiek ’s permission is to strip back the unnecessary and improve spotDL’s core functionality and enhance its simplicity. As I understand, the interfaces.md document tells contributors all they need to stick to the interfaces and ABC’s don’t enforce the interfaces, ABC don’t cede that much control. I believe that If spotDL had 100’s of interfaces that manually checking for interface enforcement would not be an option, ABC’s would be sensible. I’d rather spare future contributors the trouble of handling weird behavior’s ABC’s throw up. If there are specific ways you believe ABC’s will help please do let us know. |
Some users might even cringe at its simplicity
Got the CLI up and running Updated readme and stuff... I'm up for a longgg break.
Much tighter, tidier search interface
I filled up PURPOSES.md for all of the code written, had to delete some very big functions (lots of erroft wasted) and a simple cmd line utility to ensure line count doesn't excede 200 lines
Managing to get the progress bar to sync across multiple processes was a nightmare. the download should be more reliable and less breaky now. It shill could do with a little rewriting. I'll do that asap.
|
Hey @ritiek could you pleas go through the code again? |
There are oddball cases with inexplicable errors and fluctation (pumping) of audio loudness. All those fixed
There were a few 1-line errors that could grind the download process to a halt. Fixed those. Refactored songObj -> SongObj across the codebase to keep in line with the naming convention. Started working on the docs.
EDM-like songs get their last 1-2 seconds clipped/cut/deleted by ffmpeg. we counter this here.
You can use this now.
ghost
marked this pull request as ready for review
and also added error handling for those rare cases where you don't get a youTube match at all. P.S. Its ironic that a 'patch' - something that fixes errors actually causes them but then, whatever...
Songs used to display the wrong song length, but no more, see comment at spotdl\download\downloader.py:145 for more details.
A fresh CONTRIBUTING.md and minor README changes to notify potensial contributors of the same.
…into reStructure/reCode
Spotdl is currently a sprawling codebase, this is an attempt to simplify it ground up and make it easier and more simple to contribute to.
If you see this message, I humbly ask that you spare an hour or two to look through the code.
Ofc not (even I wouldn't spend an hour or two...)
Please do look through the the 'Working Docs' and 'Temp/spotdl' folders and leave your thoughts on: