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

[python-package] drop Python 3.6 support, use dataclasses #5765

Closed
wants to merge 4 commits into from

Conversation

jameslamb
Copy link
Collaborator

Contributes to #3756.
Contributes to #3867.
Inspired by discussions in #5672 (thanks @IdoKendo !).

This PR proposes the following:

  • replacing two uses of collections.namedtuple in the Python package with dataclasses.dataclass
  • dropping support for Python 3.6

Why use dataclasses?

dataclasses are preferable to namedtuple in the following ways:

  • easier to set type annotations + default values than named tuples
    • makes it easier for both human developers and and tools like mypy to understand the expected types of properties
  • possible to attach other methods or computed properties with @property (just like normal classes)

Why drop support for Python 3.6?

dataclasses has been a part of the Python standard library since Python 3.7 (https://docs.python.org/3/library/dataclasses.html).

Python 3.6 reached end-of-life (stopped receiving even security fixes) in December 2021 (https://devguide.python.org/versions/).

Most of lightgbm's dependencies dropped Python 3.6 a long time ago, for example:

Comment on lines +10 to +11
if TYPE_CHECKING:
from .engine import CVBooster
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://mypy.readthedocs.io/en/stable/runtime_troubles.html#import-cycles for an explanation of this.

Adding this pattern to avoid cyclical imports... engine.CVBooster is needed for the type annotation on CallbackEnv.model, but engine.py also tries to import all of callback's contents.

from . import callback

@jmoralez
Copy link
Collaborator

jmoralez commented Mar 4, 2023

Even though some dependencies have dropped support for python 3.6, (I think) we don't rely on any new functionality from them, so if the drop is only for using dataclasses I don't think it's worth it, mainly because of the reasoning described in #5006 (comment), I know a couple of companies that are still stuck with py36 and since 4.0.0 will have many new features I don't think users should miss out on those just for the sake of type hints on a couple of data structures.

I'm not hardly against it though, if you feel it's time and think it's worth it I won't oppose.

@jameslamb
Copy link
Collaborator Author

jameslamb commented Mar 6, 2023

Thanks @jmoralez ! I think you're right, I shouldn't have coupled the "drop Python 3.6" decision together with this attempt to get slightly clearer type annotations.

I'm going to close this and open some new PRs proposing replacing these namedtuple with regular-old Python classes.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed.
To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues
including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants