Skip to content
This repository has been archived by the owner on Oct 16, 2023. It is now read-only.

Gafferpy tidy up operation parameter handling #896

Closed
t92549 opened this issue May 18, 2021 · 1 comment · Fixed by #929
Closed

Gafferpy tidy up operation parameter handling #896

t92549 opened this issue May 18, 2021 · 1 comment · Fixed by #929

Comments

@t92549
Copy link
Contributor

t92549 commented May 18, 2021

Currently in the Gafferpy codebase there is a non-pythonic pattern of processing operation parameters by doing something like this:

if conditional is not None:
if not isinstance(conditional, Conditional):
self.conditional = JsonConverter.from_json(conditional,
Conditional)
else:
self.conditional = conditional
else:
self.conditional = None

Not only is this over-complicated, but it causes bugs when one of those cases is missed (#870).
Fishbowl does not have this issue due to the nature of it having no error checking and presuming everything is json.

@sw96411 suggested an improvement on #890 (comment) which is more pythonic, easier to read and less bug prone.

@t92549
Copy link
Contributor Author

t92549 commented May 21, 2021

Also worth noting that while changing to this approach, it should be easier to avoid bugs like in #851.

@t92549 t92549 added this to the v1.17.0 milestone May 24, 2021
@t92549 t92549 changed the title Gafferpy tidy up operation parameter hadling Gafferpy tidy up operation parameter handling Jun 14, 2021
@t92549 t92549 modified the milestones: v1.17.0, v1.18.0 Jun 14, 2021
@t92549 t92549 modified the milestones: v1.18.0, v1.19.0 Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant