Version 3

[allerter]

Version 2.0.2 that was just released on PyPi introduces new features and more importantly breaking changes. If we were to follow Semantic Versioning, 2.0.2 probably should've been 3.0. Supposing we merge #156, #158, #159, and another one I have coming where Genius no longer needs a token (I should've actually implemented that in #160), what version do you think it should be? We could go for 2.1.0 which is against Semantic Versioning, or 3.0? I'd love to hear your thoughts on this.

[johnwmillr]

Thank you for bringing this to my attention, @Allerter. As is probably apparent, I have not been following any rigorous system for versioning changes. I like your suggestion of 3.0 after merging the outstanding PRs and following Semantic Versioning after that point forward.

A question on your upcoming PR though: do you intend to do away with the token entirely? This is not a direction I'd like to head in, as I'd like to use Genius's authorized API calls whenever possible.

[allerter]

I don't intend to remove the tokens entirely, but only to make them optional. As for API-only methods, there is no way to use them without a token. And for PublicAPI-only methods, it doesn't matter if there's a token. And for methods that support both APIs, this is basically what it boils down to:

if self.client_access_token is None or public_api:
    return super(PublicAPI, self).song(song_id, text_format)
else:
    return super().song(song_id, text_format)

So the PublicAPI is only used when the user explicitly sets public_api=True or they haven't set a token. Otherwise, the call goes to the API.

[johnwmillr]

Okay, I like that approach! If a user creates an instance of the Genius class without a token, will the methods that require a token still be visible to the user? I would like for them to still be accessible but raise something like TokenRequiredError to inform the user what's available to them after authenticating. If the Genius instance doesn't contain API-only methods when created without a token, the user will be less likely to know they are available in the package.

[allerter]

The TokenRequiredErro approach sounds like a good idea. We could also add a warning using warning.warn (or logging.warn if logging becomes part of the package in the future) that says something like:

You're using tokenless Genius. All calls will be made to the public API and API-only methods will be unavailable.

[johnwmillr]

PRs #156, #158, and #159 have now all been merged. Are you still developing one more before we go to 3.0?

[allerter]

Yes, the PR for making the token optional is almost ready.

[allerter]

I think we should also resolve #109 and #126.

[johnwmillr]

I agree. The VCR testing would be especially helpful.

[allerter]

There was an IndentationError in the last commit you submitted for #126 and that caused the tests to fail. Also, I replaced the Unicode.normalize with str.replace which you resolved using the master branch and that reverted my change. Aside from the IndentationError should I open a pull request for the change I mentioned above or should I just put them in #109?

[allerter]

Committing my changes using GitHub Desktop completely threw me off track. There's also another issue that the open()s that are now in types/base.py, don't have encoding=utf8.

[johnwmillr]

Shoot, sorry about those mistakes! I was a little careless while merging through the web interface. I'd suggest opening a new PR that resolves the errors you've described here. We'll merge that, then can move forward with 3.0.

What do you think?

[allerter]

Sure, that's a good approach. After merging that PR, should we also merge #109?

[johnwmillr]

Yes, let's merge #109 before doing 3.0.