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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Convert more record classes to dataclasses #12659

wants to merge 3 commits into from

Conversation

ichard26
Copy link
Member

@ichard26 ichard26 commented Apr 29, 2024 •
edited

  • 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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pradyunsg pradyunsg pradyunsg left review comments

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@ichard26 @pradyunsg