Skip to content

Updating output checking processes#1246

Merged
LFISHER7 merged 7 commits intomainfrom
updating-output-checking-processes
Aug 1, 2023
Merged

Updating output checking processes#1246
LFISHER7 merged 7 commits intomainfrom
updating-output-checking-processes

Conversation

@LFISHER7
Copy link
Contributor

@LFISHER7 LFISHER7 commented Jun 13, 2023

Adds updates to our stance on rounding and some instruction on releasing intermediate results

resolves https://github.com/ebmdatalab/service-analytics-team/issues/131
resolves https://github.com/ebmdatalab/service-analytics-team/issues/142

LFISHER7 added 2 commits June 13, 2023 11:00
Defaults to requiring rounding. Non rounded counts by exception
@HelenCEBM
Copy link
Contributor

I wonder if the intermediate data part could be a little clearer, e.g. re. the first point on changing chart labels, I would normally release the data behind a chart anyway (e.g for supplementary data) so would consider that final data rather than intermediate... I'm not sure if this is the first mention of running code outside rather than inside the server so not sure if some people might get a bit confused by that?

Co-authored-by: HelenCEBM <helen.curtis@thedatalab.org>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 20, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d3e42d6
Status: ✅  Deploy successful!
Preview URL: https://802c049c.opensafely-docs.pages.dev
Branch Preview URL: https://updating-output-checking-pro.opensafely-docs.pages.dev

View logs

@LFISHER7
Copy link
Contributor Author

I agree by our definition of final outputs on this page (as being outputs for your publication), the underlying data could be considered a final output. But it's not been common for people to request release of this. Are you able to suggest how this could be clearer?

By the time people are releasing outputs, they should be familiar with the concept of running locally but i've added in a section with some considerations for when you run locally on released outputs.

now that rounding is required, this is needed to provide equal protection to all counts.
If you have had [intermediate data released](#release-of-intermediate-data) and you wish to run further analyses on them, such as reformatting figures, there are a few things to consider.

1. You should include the code for these steps in your GitHub repo.
2. You **should not** commit any of the released outputs to your GitHub repo. Make sure to include them in the `.gitignore` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

what about the final processed charts/tables created as a result, should they be excluded from the repo as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they have to be, for now, given our approaches to making repos public and the template readme content. Ultimately, we'll need to be able to upload results to the jobs site

Copy link
Contributor

Choose a reason for hiding this comment

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

OK - should we add a note here so it's clear?

Copy link
Contributor

@HelenCEBM HelenCEBM left a comment

Choose a reason for hiding this comment

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

Just one question

@LFISHER7 LFISHER7 merged commit 5af1a40 into main Aug 1, 2023
@LFISHER7 LFISHER7 deleted the updating-output-checking-processes branch August 1, 2023 11:55
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.

2 participants