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

Major refactor of code to use HazenTask classes #266

Merged
merged 46 commits into from
Sep 20, 2022
Merged

Conversation

laurencejackson
Copy link
Collaborator

@laurencejackson laurencejackson commented Aug 31, 2022

Following work to separate the webapp and hazenlib functionality of the hazen project the decision was made to take this opportunity to standardise hazen tasks.

This led to the introduction of the HazenTask class, all tasks that hazen performs will inherit from this superclass leading to a number of benefits:

  • allows us to easily add new tasks without rewriting boilerplate on new tasks
  • the webapp is able to automatically find relevant tasks by searching for HazenTask objects
  • reduces code duplication
  • standardised data input/output across all tasks using HazenTask
  • separating the webapp and hazenlib features massively simplifies the repository install process for most users

Currently, all MagNet object tasks currently performed as part of routine QA at GSTT have been refactored to use the HazenTask syntax. The following tasks have not been converted to the HazenTask syntax but are still available on the CLI interface:

snr_map @pcw24601
relaxometry @pcw24601
acr_uniformity @YassineRMH

These tasks should be converted to the new syntax when possible to allow them to be accessible on the web frontend.

Laurence Jackson and others added 30 commits July 21, 2022 10:37
@github-actions
Copy link

github-actions bot commented Aug 31, 2022

Comment on lines 21 to 41

# # Service containers to run with `container-job`
# services:
# # Label used to access the service container
# postgres:
# # Docker Hub image
# image: postgres
# # Provide the password for postgres
# ports:
# - 5432:5432
# env:
# POSTGRES_DB: hazen_test
# POSTGRES_USER: test_user
# POSTGRES_PASSWORD: test_user_password
# # Set health checks to wait until postgres has started
# options: >-
# --health-cmd pg_isready
# --health-interval 10s
# --health-timeout 5s
# --health-retries 5

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is commented out in tests_development.yml but not tests_release.yml – is that intentional or an oversight?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oversight, will correct now

* `Jane Ansell <https://github.com/ansellj>`_
* `Jamie Small <https://github.com/JamieSmall>`_
* `Elizabeth Stamou <https://github.com/elizaGSTT>`_
* `Elizabeth Stamou <https://github.com/elizaGSTT>`_
Copy link
Collaborator

Choose a reason for hiding this comment

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

So good we included her twice

@tomaroberts
Copy link
Collaborator

tomaroberts commented Sep 20, 2022

Tested:

  • CLI version installed via PyPI – worked
  • CLI version installed via pip install -r requirements.txt - worked and unit tests successful (albeit with some usual warnings)
  • new --output option – worked

Broadly reviewed other files.

Good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants