joalla / discogs_client

Continuation of the "Official Python Client for the Discogs API"
https://python3-discogs-client.readthedocs.io
Other
310 stars 50 forks source link

Small fixes for Python 3.12 references #140 #141

Closed arogl closed 1 month ago

arogl commented 11 months ago

Fixes #140

I did not want to drop earlier python versions.

The dateutil package is awaiting a maintainer release dateutil#1314

JOJ0 commented 11 months ago

Hi @arogl 👋 thanks!

I think it's safe to say we should finally ditch 3.6 and 3.7

Please remove and then we'll let the workflows run and see what happens :-)

arogl commented 11 months ago

Hi @arogl 👋 thanks!

I think it's safe to say we should finally ditch 3.6 and 3.7

Please remove and then we'll let the workflows run and see what happens :-)

The build could also reference the 3.x from beets

jobs:
  test:
    runs-on: ${{ matrix.platform }}
    strategy:
      matrix:
        platform: [ubuntu-latest, windows-latest]
        python-version: ['3.7', '3.8', '3.9', '3.x']
JOJ0 commented 10 months ago

I did some fixes for our docs build, thus merged in master and resolving conflict here.

I'm not sure how we now should fix the issue that import enum is not possible for older Python versions (3.8., 3.9.). Any idea @arogl ?

arogl commented 10 months ago

I did some fixes for our docs build, thus merged in master and resolving conflict here.

I'm not sure how we now should fix the issue that import enum is not possible for older Python versions (3.8., 3.9.). Any idea @arogl ?

@JOJ0 I think it will need to be a try/except, or test version >= 3.12.

arogl commented 10 months ago

@JOJ0 I am looking at the latest failure and my simple python searching shows I may need to duplicate the function, in an if-else wrapper due to the @enum.member decorators not existing in versions of python before 3.12.

Thoughts?

AnssiAhola commented 10 months ago

@JOJ0 I am looking at the latest failure and my simple python searching shows I may need to duplicate the function, in an if-else wrapper due to the @enum.member decorators not existing in versions of python before 3.12.

Thoughts?

@arogl @JOJ0 If I can chime in 🙂 You could try something like importing the enum.member in a try/except block and fallback to a "empty" decorator, this way you won't have to declare the function twice with if statements

try:
   from enum import member
except ImportError:
   member = no_op_decorator // this would be an decorator defined elsewhere that does nothing

@member
class By(Enum):

the no_op_decorator could be declared in the utils.py file

def no_op_decorator():
    pass

Note. this code isn't tested

JOJ0 commented 10 months ago

Oh @AnssiAhola that sounds like a quite clean to read solution. Great idea. Give that a try @arogl

Let me think about where such a helper def should live...

AnssiAhola commented 10 months ago

Let me think about where such a helper def should live...

Could also just declare the empty decorator in the except block? If python allows that. This way it's not necessary to keep it in some random helper/utils file.

...
except ImportError:
    def member():
        pass
AnssiAhola commented 10 months ago

Seems like it still fails, because some arguments are being passed to the empty decorator. Hmm...

Maybe try adding *args and **kwargs as parameters.

def member(*args, **kwargs):
    pass
AnssiAhola commented 10 months ago

Although that might still fail, because we need to return the function that is passed to the decorator 🤔 If that doesn't work try

def member(func):
    return func

Sorry, not on my computer so cant test it myself, hence the trial and error 😅

arogl commented 10 months ago

Although that might still fail, because we need to return the function that is passed to the decorator 🤔 If that doesn't work try

def member(func):
    return func

Sorry, not on my computer so cant test it myself, hence the trial and error 😅

Second attempt ☹

AnssiAhola commented 10 months ago

Wohoo, we have a runner!

JOJ0 commented 1 month ago

I'm just not sure what i could do about the kind of "hanging" build (3.6/3.7) CI checks. To trigger a re-run I'll now close and reopen the PR.

JOJ0 commented 1 month ago

Ok I assume this is a bug. It should go away in future CI runs since we kicked out 3.6 and 3.7 build with this one.

JOJ0 commented 1 month ago

Ok I assume this is a bug. It should go away in future CI runs since we kicked out 3.6 and 3.7 build with this one.

Yep, all green in master now :-)