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

Convert DAG node classes to dataclasses #1320

Merged
merged 6 commits into from
Jul 11, 2024
Merged

Conversation

plypaul
Copy link
Contributor

@plypaul plypaul commented Jul 10, 2024

  • Previously, the DAG-node classes in MF were not dataclasses due to issues with class inheritance. These have since been resolved via hierarchy changes, so this PR updates those classes to be dataclasses. In general, dataclasses make these easier to use, and there are upcoming use cases where dataclasses will simplify implementation (e.g. graph component comparison, serialization).
  • A create() method was added to simplify many initialization use cases while not overriding the one generated by dataclasses.
  • There is an update to how the node_id field is set - please see mf_dag.py.
  • Otherwise, this should be a mechanical update with no substantive logic changes.
  • There are no snapshot changes, so that should simplify review.
  • Please view by commit.

@cla-bot cla-bot bot added the cla:yes label Jul 10, 2024
Copy link

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@plypaul plypaul force-pushed the p--query-resolution-perf--02 branch from 34df6c4 to 1f72044 Compare July 10, 2024 20:48
@plypaul plypaul force-pushed the p--query-resolution-perf--03 branch from b68a0fd to cf3666e Compare July 10, 2024 20:48
@plypaul plypaul force-pushed the p--query-resolution-perf--03 branch 2 times, most recently from 2bd689d to 2164fe0 Compare July 10, 2024 20:55
@plypaul plypaul marked this pull request as ready for review July 10, 2024 20:59
@plypaul plypaul force-pushed the p--query-resolution-perf--03 branch 2 times, most recently from 53ca214 to 54f7942 Compare July 10, 2024 21:39
@plypaul plypaul changed the title Convert DAG node classes to be dataclasses Convert DAG node classes to dataclasses Jul 10, 2024
Copy link
Contributor

@courtneyholcomb courtneyholcomb left a comment

Choose a reason for hiding this comment

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

LGTM!

@plypaul plypaul force-pushed the p--query-resolution-perf--02 branch 2 times, most recently from fbb645c to f29f6f6 Compare July 11, 2024 16:59
Base automatically changed from p--query-resolution-perf--02 to main July 11, 2024 17:20
@plypaul plypaul force-pushed the p--query-resolution-perf--03 branch from 54f7942 to 09049a7 Compare July 11, 2024 17:22
@plypaul plypaul merged commit 072b8d5 into main Jul 11, 2024
15 checks passed
@plypaul plypaul deleted the p--query-resolution-perf--03 branch July 11, 2024 17:27
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.

2 participants