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

Cancer_modules_depression_epilepsy convert excel files to csv files #1487

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

mnjowe
Copy link
Collaborator

@mnjowe mnjowe commented Oct 10, 2024

This PR aims at converting excel files in all cancer modules plus epilepsy and depression modules

Person responsible - Andrew Phillips

@mnjowe mnjowe self-assigned this Oct 10, 2024
@mnjowe mnjowe added the epi label Oct 10, 2024
@mnjowe
Copy link
Collaborator Author

mnjowe commented Nov 6, 2024

Hi @tbhallett. You may want to do a final review on this PR and merge when happy. I have already tested hashing DataFrames from both master and this branch. They are all outputting same hash

Thanks

@joehcollins joehcollins self-requested a review November 7, 2024 12:18
@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:23f3b4cef6927114b414344c95a4c262691312049ce7dc101042230e895ff066
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to keep this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@andrew-phillips-1 , should I delete this file?

@@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:e5f0e3c22d3b82061dc884df42f3a304dd0cd1b205166d1e5460c8569eba0d53
Copy link
Collaborator

Choose a reason for hiding this comment

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

similarly if this is just documentation r.e. assumptions maybe it doesnt need to live on master?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should I delete it then? I will defer to you or @andrew-phillips-1 for a final decision

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @mnjowe It's fine to delete from here so long as we have a central copy kept somewhere. I can't recall where the write-ups are held.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

great, thanks @andrew-phillips-1. For all the files I have converted, original copies are stored in our TLO sharepoint here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @mnjowe

@tamuri
Copy link
Collaborator

tamuri commented Dec 17, 2024

Some files here have also been changed in #1487. Is this PR based off that one?

@mnjowe
Copy link
Collaborator Author

mnjowe commented Dec 18, 2024

Hi Asif @tamuri. That link points to this same PR BUT I agree there might be that possibility of one excel file re-converted in another PR. The reason being I have had more PR's on converting excel files to CSV branching off master.

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

Successfully merging this pull request may close these issues.

4 participants