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.

Sign up for GitHub

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

Jump to bottom

Convert more record classes to dataclasses #12659

Merged
merged 6 commits into from
Dec 7, 2024
Merged

Convert more record classes to dataclasses #12659

merged 6 commits into from
Dec 7, 2024

Conversation

ichard26
Copy link
Member

@ichard26 ichard26 commented Apr 29, 2024 •
edited
Loading

  • Removes BestCandidateResult's iter_all() and iter_applicable() methods as they were redundant
  • Removes ParsedLine's is_requirement attribute as it was awkward to use (to please mypy, you would need to add asserts on .requirement)

@ichard26 ichard26 added the type: refactor Refactoring code label Apr 29, 2024
@ichard26 ichard26 force-pushed the moar-dataclasses branch 2 times, most recently from 9eb70f9 to 748aff7 Compare April 29, 2024 20:37
ichard26 added 3 commits May 5, 2024 12:12
@ichard26
Convert more record classes to dataclasses
c4f08e0 
- Removes BestCandidateResult's iter_all() and iter_applicable()
  methods as they were redundant

- Looking closer at SelectionPreferences, it only used slots to prevent
  accidental new attribute assignment (not for reduced memory usage as I
  previously thought) so this is safe to convert

- Removes ParsedLine's is_requirement attribute as it was awkward to use
  (to please mypy, you would need to add asserts on .requirement)
@ichard26
Revert SelectionPreferences as it's performance-sensitive
40a9a20
@ichard26
Convert FrozenRequirement
d05323c
@ichard26 ichard26 force-pushed the moar-dataclasses branch from 8738ed3 to d05323c Compare May 5, 2024 16:14
@ichard26 ichard26 added this to the 24.2 milestone Jun 3, 2024
@ichard26 ichard26 removed this from the 24.2 milestone Jul 2, 2024
@ichard26 ichard26 requested a review from pradyunsg July 4, 2024 02:17
@ichard26
Copy link
Member Author

@pradyunsg can I kindly request another review from you? :)

(this is a very low priority PR, so please spend your time elsewhere if needed)

pradyunsg
pradyunsg approved these changes Jul 18, 2024
View reviewed changes
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

LGTM, with a minor suggestion.

src/pip/_internal/req/req_file.py Show resolved Hide resolved
src/pip/_internal/req/req_file.py Show resolved Hide resolved
@ichard26
Copy link
Member Author

I've added slots for those two requirement file dataclasses. Unfortunately, dataclass(slots=True) is a Python 3.10+ feature, so I've manually set __slots__ while removing the defaults as they conflict.

@ichard26
Copy link
Member Author

ichard26 commented Aug 9, 2024 •
edited
Loading

*bump*

This PR is a pure refactoring change and is low-risk. It should be trivial to review.

P.S. If you find my bumps annoying, please let me know and I will stop. I just hate seeing otherwise good PRs never land because they're so old that no one sees them.

@ichard26 ichard26 added this to the 25.0 milestone Nov 8, 2024
@ichard26 ichard26 merged commit 8dbbb2e into pypa:main Dec 7, 2024
32 checks passed
@ichard26 ichard26 deleted the moar-dataclasses branch December 7, 2024 20:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pradyunsg pradyunsg pradyunsg approved these changes

Assignees
No one assigned
Labels
bot:chronographer:provided type: refactor Refactoring code
Projects
None yet
Milestone
25.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ichard26 @pradyunsg