Skip to content
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

Should we split InfoExtractors.py? #846

Closed
FiloSottile opened this issue May 20, 2013 · 5 comments
Closed

Should we split InfoExtractors.py? #846

FiloSottile opened this issue May 20, 2013 · 5 comments

Comments

@FiloSottile
Copy link
Collaborator

@FiloSottile FiloSottile commented May 20, 2013

With gen_extractors, get_info_extractor and the package layout will be trivial to split the IEs in per-site files.

Do you feel like it's a good idea? I start to lose myself in that 4000 LoC.

@FiloSottile
Copy link
Collaborator Author

@FiloSottile FiloSottile commented May 20, 2013

(pinging devs @phihag @jaimeMF)

@phihag
Copy link
Contributor

@phihag phihag commented May 20, 2013

Yes, that's as it was always intended. Feel free to move out ie's to youtube_dl/extractors/<ie-id>.py. The common extractor infrastructure should be then moved to youtbe_dl/extractors/generic.py.

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented May 20, 2013

+1, the only downside is that (if I'm not wrong) adding a new IE would require importing the file.

@FiloSottile
Copy link
Collaborator Author

@FiloSottile FiloSottile commented May 20, 2013

I have to do some tests and see the package importing capabilities...

Filippo Valsorda

2013/5/20 Jaime Marquínez Ferrándiz notifications@github.com

+1, the only downside is that (if I'm not wrong) adding a new IE would
require importing the file.


Reply to this email directly or view it on GitHubhttps://github.com//issues/846#issuecomment-18144453
.

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented Aug 30, 2013

This has been done and it's working great, now there are over 9000 LOC although it includes the tests) inside extractor, having all in a single file would be insane.

@jaimeMF jaimeMF closed this Aug 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.