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

Add storage and system card service for writing results away #24

Closed
wants to merge 28 commits into from

Conversation

laurensWe
Copy link
Member

@laurensWe laurensWe commented May 28, 2024

Description

Two services are added, one Storage service which handles writing away of .yaml files to different storage locations the second service is the basic implementation of a system card to be extended for full system card functionality.

Link all GitHub issues fixed by this PR.

Resolves #

Checklist

Please check all the boxes that apply to this pull request using "x":

  • I have tested the changes locally and verified that they work as expected.
  • I have followed the project's coding conventions and style guidelines.
  • I have rebased my branch onto the latest commit of the main branch.
  • I have squashed or reorganized my commits into logical units.
  • I have read, understood and agree to the Developer Certificate of Origin, which this project utilizes.

@laurensWe laurensWe requested a review from a team as a code owner May 28, 2024 13:16
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to our community 🤗 and thank you for your first contribution.

As a first time contributor please make sure to review our contribution guidelines ❤️

pyproject.toml Outdated Show resolved Hide resolved
@laurensWe laurensWe force-pushed the write-done-task-to-system-card branch from 3842756 to 9200036 Compare May 28, 2024 13:57
@laurensWe laurensWe force-pushed the write-done-task-to-system-card branch from 9200036 to 6706ba9 Compare May 28, 2024 13:59
from yaml import safe_load


@pytest.mark.skip(reason="This is an initialisation function for the tests")
Copy link
Member

Choose a reason for hiding this comment

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

I believe fixtures are meant for this kind of functionalty, or is there a reason to do it like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Changed this to use fixtures

Copy link
Member

@anneschuth anneschuth left a comment

Choose a reason for hiding this comment

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

Nice to break things up! Curious what the next step is.
System card validation?
Updates that supports system card paths (as they are defined in the instrument register)?

@uittenbroekrobbert uittenbroekrobbert force-pushed the setup-mvp-cards branch 2 times, most recently from fccce5c to d8f7b92 Compare May 29, 2024 13:31
@uittenbroekrobbert
Copy link
Contributor

I first rebased this branch (locally) onto the setup-mvp-cards branch, as that is ready to merge into main. Due to those changes, I believe some tests don't work anymore. I'll look into this.

@uittenbroekrobbert uittenbroekrobbert force-pushed the setup-mvp-cards branch 2 times, most recently from b211a4f to ac57364 Compare June 3, 2024 10:20
@berrydenhartog berrydenhartog mentioned this pull request Jun 3, 2024
5 tasks
@laurensWe laurensWe closed this Jun 5, 2024
@laurensWe
Copy link
Member Author

Closed because it is duplicate of #30

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.

4 participants