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

Create 2021_Posth_SciAdv.janno #204

Conversation

EleniSef
Copy link
Contributor

@EleniSef EleniSef commented Aug 15, 2024

.janno file for future use.

Posth et al. Sci Adv The origin and legacy of the Etruscans through a 2000-year archeogenomic time transect 2021 https://www.science.org/doi/10.1126/sciadv.abi7673

@nevrome nevrome added the only .janno This PR does not feature a full package, but only a .janno file label Aug 18, 2024
@stschiff
Copy link
Member

Brillant @EleniSef, thanks for submitting. We'll take a look.

@nevrome nevrome self-assigned this Sep 6, 2024
@nevrome
Copy link
Member

nevrome commented Sep 6, 2024

Thanks for preparing this .janno file! I see the following issues

  • The package name does not follow our expected standard of Year_AuthorName_RelevantKeyword. I propose 2021_Posth_Etruscans.
  • Please remove all columns that are completely empty/filled only with n/a.
  • The underscores in the location (e.g. Siena_Tuscany), site name or the free text/note columns are not necessary. Spaces are fine. I think something like Siena (Tuscany) would be more natural for the locations.
  • Some lat/lon coordinates are unreasonably precise - up to 7 decimal places, which translates to a precision of about 1cm at the equator. 5 decimal places are sufficient.
  • The term Chiostraccio man for sample UDC_P sounds like a prime example for an entry in the Alternative_IDs column. I would remove it from the Collection_ID and move it there.
  • The entries in Alternative_IDs are often duplicates of the Poseidon_ID. If they're the same, they can be removed.
  • The numbers in Coverage_on_Target_SNPs are probably meant to be entries in Nr_SNPs, right?

Maybe you could quickly have a look 👍

@nevrome
Copy link
Member

nevrome commented Sep 6, 2024

@EleniSef Remember that you can just modify the data on this branch and do not have to open a new PR. Let me know if this process is unclear.

@EleniSef
Copy link
Contributor Author

@nevrome Done!
I have made all my changes and then committed the changes. Let me know if you have everything you need.

@stschiff
Copy link
Member

That is so useful, @EleniSef, as I literally a few days ago received the genotype data from Cosimo. So we can turn this into a package real quick now! Thanks a lot.

@TCLamnidis
Copy link
Member

@stschiff Where is the genotype data? Should we turn this into a full package?

@stschiff
Copy link
Member

Yes, that's on my list. Hope to update this soon.

@stschiff
Copy link
Member

So, thanks again to @EleniSef for preparing this and @nevrome for going through. I will now merge this Janno into a dedicated internal branch (add_2021_Posth_SciAdv), so that I can add the genotype data and move this forward.

@stschiff stschiff changed the base branch from dev to add_2021_Posth_SciAdv October 11, 2024 10:23
@stschiff
Copy link
Member

(And I will make sure @EleniSef gets credited in the contributor fields, of course!)

@stschiff stschiff merged commit 6267a7f into poseidon-framework:add_2021_Posth_SciAdv Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
only .janno This PR does not feature a full package, but only a .janno file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants