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

Remove arrow depencency #38

Closed
netrounds-fredrik opened this issue Feb 13, 2015 · 3 comments · Fixed by #40
Closed

Remove arrow depencency #38

netrounds-fredrik opened this issue Feb 13, 2015 · 3 comments · Fixed by #40

Comments

@netrounds-fredrik
Copy link
Contributor

I think this looks like a nice and simple library for parsing m3u8 playlists, but one thing that kind of ruined that impression was when I realized that it depended on arrow. I'm not saying arrow is a bad library, but it's way to big (with all it's own dependencies) for what it's used for in this project.

Currently arrow is used for two operations:

  • One in parser.py: arrow.get(value).datetime...
  • ... and one in models.py: arrow.get(self.program_date_time).isoformat()

So it's basically parsing and formatting date and times in ISO 8601 format. A more lightweight library without additional dependencies for parsing ISO 8601 is https://pypi.python.org/pypi/iso8601. The operations currently done with arrow can be done like this with iso8601:

  • iso8601.parse_date(value)
  • self.program_date_time.isoformat()

Completely removing parsing (just providing the value as a string) or making it optional is another way to go.

@flavioribeiro
Copy link
Contributor

@netrounds-fredrik +1 on this. Would you mind sending a pull request?

@netrounds-fredrik
Copy link
Contributor Author

Created a pull request. All unit tests passes.

flavioribeiro added a commit that referenced this issue Mar 3, 2015
Replace arrow with iso8601 that is more lightweight (fixes #38)
@netrounds-fredrik
Copy link
Contributor Author

I see that arrow has been reintroduced as dependency for the unit tests in f50cad5. What was the reason for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants