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 deprecated functions and classes #8409

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

DanielNoord
Copy link
Collaborator

Type of Changes

Type
βœ“ πŸ”¨ Refactoring
βœ“ πŸ“œ Docs

Description

We're getting there.

@DanielNoord DanielNoord added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Mar 8, 2023
@DanielNoord DanielNoord added this to the 3.0.0 milestone Mar 8, 2023
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #8409 (3b4d040) into main (0db9441) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8409    /-   ##
=======================================
  Coverage   95.68%   95.68%           
=======================================
  Files         176      175    -1     
  Lines       18504    18465   -39     
=======================================
- Hits        17705    17668   -37     
  Misses        799      797    -2     
Impacted Files Coverage Ξ”
pylint/checkers/__init__.py 93.93% <ΓΈ> (-0.18%) ⬇️
pylint/checkers/utils.py 96.00% <ΓΈ> ( 0.15%) ⬆️
pylint/lint/expand_modules.py 95.18% <ΓΈ> (-0.23%) ⬇️
pylint/lint/utils.py 95.83% <ΓΈ> (-0.72%) ⬇️
pylint/utils/__init__.py 100.00% <ΓΈ> (ΓΈ)
pylint/lint/__init__.py 92.30% <100.00%> (ΓΈ)
pylint/utils/utils.py 86.78% <100.00%> (-0.23%) ⬇️

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.

We need to explain what's the alternative, for example "To make a checker reduce map data simply implement get_map_data and reduce_map_data." etc. etc. (We can copy paste from the deprecation message and adapt).

A number of old utility functions and classes have been removed.
These are ``MapReduceMixin``, ``is_inside_lambda``, ``check_messages``,
``is_class_subscriptable_pep585_with_postponed_evaluation_enabled``,
``get_python_paht``, ``fix_import_path`` and ``get_global_option``.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
``get_python_paht``, ``fix_import_path`` and ``get_global_option``.
``get_python_path``, ``fix_import_path`` and ``get_global_option``.

@DanielNoord
Copy link
Collaborator Author

We need to explain what's the alternative, for example "To make a checker reduce map data simply implement get_map_data and reduce_map_data." etc. etc. (We can copy paste from the deprecation message and adapt).

I was wondering about that. It feels as if the changelog can become very long. Shouldn't we just refer to the code and let the old DeprecationWarnings show what needs to be changed?

@Pierre-Sassoulas
Copy link
Member

Yeah it's going to be long but it's the place where people are going to look first when something break, so it needs to explain what to do imo (also if someone is taking care of the deprecation warnings they won't need it).

@github-actions

This comment has been minimized.

@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0, 3.0.0a6 Mar 9, 2023
@github-actions

This comment has been minimized.

@DanielNoord
Copy link
Collaborator Author

@Pierre-Sassoulas LGTM!

@DanielNoord
Copy link
Collaborator Author

But docs fail :(

@github-actions

This comment has been minimized.

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.

I don't think we can use enumeration in the fragments, because it's already inside an enumeration. It's probably fixable but also not worth imo.

@Pierre-Sassoulas Pierre-Sassoulas merged commit b2ab82a into pylint-dev:main Mar 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on django:
The following messages are now emitted:

  1. redefined-variable-type:
    Redefinition of field type from django.db.models.fields.related.OneToOneField to django.forms.widgets.Widget
    https://github.com/django/django/blob/32d4b61c313be5169137047e9fb3668da20a5d89/django/db/models/base.py#L346

Effect on music21:
The following messages are now emitted:

  1. invalid-name:
    Attribute name "id" doesn't conform to '[a-z_][A-Za-z0-9_]{2,30}$' pattern
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/base.py#L576
  2. useless-suppression:
    Useless suppression of 'attribute-defined-outside-init'
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/base.py#L3193
  3. useless-suppression:
    Useless suppression of 'attribute-defined-outside-init'
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/base.py#L3264
  4. redefined-variable-type:
    Redefinition of fundamental type from django.forms.widgets.Widget to music21.pitch.Pitch
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/pitch.py#L3690
  5. too-many-instance-attributes:
    Too many instance attributes (33/20)
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/roman.py#L1394
  6. redefined-variable-type:
    Redefinition of lastChord type from music21.chord.Chord to django.forms.widgets.Widget
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/analysis/neoRiemannian.py#L435
  7. redefined-variable-type:
    Redefinition of n type from music21.note.Note to music21.note.GeneralNote
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/abcFormat/translate.py#L354
  8. redefined-variable-type:
    Redefinition of sc3 type from music21.scale.MinorScale to music21.scale.MajorScale
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/scale/test_scale_main.py#L163
  9. useless-suppression:
    Useless suppression of 'method-hidden'
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/stream/base.py#L11551

The following messages are no longer emitted:

  1. invalid-name:
    Attribute name "id" doesn't conform to '[a-z_][A-Za-z0-9_]{2,30}$' pattern
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/prebase.py#L293
  2. suppressed-message:
    Suppressed 'attribute-defined-outside-init' (from line 3193)
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/base.py#L3193
  3. suppressed-message:
    Suppressed 'attribute-defined-outside-init' (from line 3264)
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/base.py#L3264
  4. too-many-instance-attributes:
    Too many instance attributes (34/20)
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/roman.py#L1394
  5. redefined-variable-type:
    Redefinition of sc3 type from music21.scale.MajorScale to music21.scale.MinorScale
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/scale/test_scale_main.py#L169
  6. suppressed-message:
    Suppressed 'method-hidden' (from line 11551)
    https://github.com/cuthbertLab/music21/blob/aacc7aa37552f216a698d8af9e5ea3c685cf8cc2/music21/stream/base.py#L11552

Effect on pandas:
The following messages are now emitted:

  1. redefined-variable-type:
    Redefinition of expected type from pandas.core.arrays.period.PeriodArray to pandas.core.indexes.base.Index
    https://github.com/pandas-dev/pandas/blob/30992187146e923623bfc90f9dbb49c2c3cce5fb/pandas/_testing/__init__.py#L300

The following messages are no longer emitted:

  1. redefined-variable-type:
    Redefinition of expected type from pandas.core.arrays.numpy_.PandasArray to pandas.core.indexes.base.Index
    https://github.com/pandas-dev/pandas/blob/30992187146e923623bfc90f9dbb49c2c3cce5fb/pandas/_testing/__init__.py#L300

This comment was generated for commit 3b4d040

@DanielNoord DanielNoord deleted the deprecate branch March 9, 2023 13:12
toofar added a commit to qutebrowser/qutebrowser that referenced this pull request Oct 11, 2023
Changelog: https://pylint.pycqa.org/en/latest/whatsnew/3/3.0/index.html#summary-release-highlights

remove `__implements__`:
That attribute is apparently from a rejected PEP. They say to inherit
from BaseChecker, which we are already doing.
pylint-dev/pylint#8404

check_messages -> only_required_for_messages:
Seems straightforward instructions. I haven't actually tested it.
pylint-dev/pylint#8409

remove emptystring extension:
Looks like this has been replaced by https://pylint.readthedocs.io/en/stable/user_guide/messages/convention/use-implicit-booleaness-not-comparison-to-string.html
Which is disabled by default but we have `enable=ALL`, so I guess that means
we indeed have it enabled.

And update changelog URLs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants