Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

remove dataclasses_json #817

Merged
merged 1 commit into from
Jul 25, 2023
Merged

remove dataclasses_json #817

merged 1 commit into from
Jul 25, 2023

Conversation

dperl-dls
Copy link
Collaborator

Fixes #815

To test:

  1. Run tests

Copy link
Collaborator

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks. Query in code, take it or leave it as part of this issue

@@ -18,7 +17,7 @@


@dataclass
class GridScanWithEdgeDetectParams(DataClassJsonMixin, AbstractExperimentParameterBase):
class GridScanWithEdgeDetectParams(AbstractExperimentParameterBase):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Query: RotationScanParams inherits from BaseModel. Do we have a preference on when to do that and when to use pydantic.dataclass? From a google I think maybe BaseModel gives you more? So I would suggest we standardise on them all using BaseModel? This may be a different ticket...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, maybe it's better to use models for all of them, I just changed the ones which were still dataclass because it was easier

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, I'll make an issue

@DominicOram DominicOram mentioned this pull request Jul 24, 2023
@dperl-dls dperl-dls merged commit 3ea6e6f into main Jul 25, 2023
10 checks passed
@dperl-dls dperl-dls deleted the 815_remove_dataclassesjson branch July 25, 2023 12:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove dataclasses_json (and maybe dataclasses)
2 participants