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

Support for python 3.12 datetime #427

Merged

Conversation

matejsp
Copy link
Contributor

@matejsp matejsp commented Feb 11, 2024

Fixes #425

@coveralls
Copy link

coveralls commented Feb 11, 2024

Pull Request Test Coverage Report for Build 8029201136

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build 6757623180: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

If I understand correctly the best case scenario is for python 3.12 (faster because there is no error raised then caught) ? With only 7.5% users on it, it's not the most likely interpreter for pylint-django users so it should be the other way around see https://pypistats.org/packages/pylint-django.

daily_download_proportion_pylint_django

Also, maybe it would be better to have a "is_python_3_12" constant than catch the problem every-time ?

@matejsp
Copy link
Contributor Author

matejsp commented Feb 24, 2024

@Pierre-Sassoulas

When creating the code I was trying to replicate the existing code with _pydecimal that uses the same try catch.

I was testing the following fix in our monolit with 500 kLOC and more than 700 db tables and around the same number of datetime fields and there wasnt any negative performance impact.

But I see your point and wish your concerns would be shared with pylint developers :D We only run on CI on AWS and in our case it takes 7 minutes with 12 core on very large ec2 instance to lint 500 kLOC code and on my computer it takes about an hour. Pylint lack of performance and even on home page bragging how painfully slow pylint is, are making our developers cry. Otoh mypy takes about 1 minute.

I will refactor based on your proposal with a constant.

@matejsp matejsp force-pushed the fix-datetime-module-import branch 2 times, most recently from 8d10bb6 to b60fc57 Compare February 24, 2024 07:11
@matejsp
Copy link
Contributor Author

matejsp commented Feb 24, 2024

Pushed the changes. CI passes.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

LGTM !

But I see your point and wish your concerns would be shared with pylint developers :D We only run on CI on AWS and in our case it takes 7 minutes with 12 core on very large ec2 instance to lint 500 kLOC code and on my computer it takes about an hour. Pylint lack of performance and even on home page bragging how painfully slow pylint is, are making our developers cry. Otoh mypy takes about 1 minute.

As far as I remember I'm the one who wrote the readme. It should not be a bragging tone (suggestions welcome to change that btw :) ). We're playing with the cards and assumptions that were made. pylint is not using the typing and infer the typing of something by checking the values in all the calls : it's not fast, it's also hard to parallelize and we're based on the python builtin ast which has its advantages but also can't be ported to rust or C (and most contributors know python anyway). There's also organically slow check like duplicate-code.. The base problem is not an easy problem to fix, and pylint being fast is not the reason people use pylint, so we focus mainly on keeping up with python development which is already a lot for a small team of volunteers. All that to say that we do share a concern about performances and try to do our best.

@Pierre-Sassoulas Pierre-Sassoulas merged commit a2b4459 into pylint-dev:master Feb 26, 2024
28 checks passed
@Pierre-Sassoulas
Copy link
Member

Just a heads up, I don't know when this will be released, you can use the github repo directly by using pip's git+ssh feature meanwhile.

@matejsp matejsp deleted the fix-datetime-module-import branch February 27, 2024 08:24
@matejsp
Copy link
Contributor Author

matejsp commented Feb 27, 2024

Thank you for the merge. I am looking forward to the official release but will test it locally for now. On CI we would need oficial release.

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

Successfully merging this pull request may close these issues.

Issue with python 3.12 and date and datetime fields.
3 participants