Skip to content

Conversation

@Tlkhi
Copy link
Contributor

@Tlkhi Tlkhi commented Aug 20, 2025

PR Checklist for a new package submission

  • The package does not exist already in the community archive, also not with a different name.
  • The package title in the POSEIDON.yml conforms to the general title structure suggested here: <Year>_<Last name of first author>_<Region, time period or special feature of the paper>, e.g. 2021_Zegarac_SoutheasternEurope, 2021_SeguinOrlando_BellBeaker or 2021_Kivisild_MedievalEstonia.
  • The package is stored in a directory that is named like the package title.

  • Samples that already have been published previously, and got re-analysed (e.g. re-sequenced) for the now packaged publication, have a modified Poseidon_ID of the form <Original Poseidon_ID>_<Initials of the main author>_<Year>. Re-analysed versions of I1685 (Lazaridis et al. 2016) should, for example, be assigned the IDs I1685_IL22 (Lazaridis et al. 2022) and I1685_IL25 (Lazaridis et al. 2025).

  • The package is complete and features the following elements:
    • Genotype data in binary PLINK format (not EIGENSTRAT format).
    • Genotype has been provided by the original authors of the publication describing the data.
    • A POSEIDON.yml file with not just the file-referencing fields, but also the following meta-information fields present and filled: poseidonVersion, title, description, contributor, packageVersion, lastModified (see here for their definition)
    • A reasonably filled .janno file (for a list of available fields look here and here for more detailed documentation about them).
    • A .bib file with the necessary literature references for each sample in the .janno file.
  • Every file in the submission is correctly referenced in the POSEIDON.yml file and there are no additional, supplementary files in the submission that are not documented there.
  • Genotype data, .janno and .bib file are all named after the package title and only differ in the file extension.
  • The package version in the POSEIDON.yml file is 1.0.0.
  • The poseidonVersion of the package in the POSEIDON.yml file is set to the latest version of the Poseidon schema.
  • The POSEIDON.yml file contains the corresponding checksums for the fields genoFile, snpFile, indFile, jannoFile and bibFile.
  • There is either no CHANGELOG file or one with a single entry for version 1.0.0.

  • The Publication column in the .janno file is filled and the respective .bib file has complete entries for the listed mentioned keys.
  • The .janno file does not include any empty columns or columns only filled with n/a.
  • The order of columns in the .janno file adheres to the standard order as defined in the Poseidon schema here.
  • The .janno and the .ssf files are not fully quoted, so they only use single- or double quotes ("...", '...') to enclose text fields where it is strictly necessary (i.e. their entry includes a TAB).

  • The package passes a validation with trident validate --fullGeno.

  • Large genotype data files are properly tracked with Git LFS and not directly pushed to the repository. For an instruction on how to set up Git LFS please look here. If you accidentally pushed the files the wrong way you can fix it with git lfs migrate import --no-rewrite path/to/file.bed (see here).

@nevrome
Copy link
Member

nevrome commented Aug 24, 2025

Thanks for preparing this - looks very well done at first glance! The only thing I noticed is the ENA: in the Genetic_Source_Accession_IDs column. The Poseidon schema only requires the ID here.

I'll try to find a reviewer for the package.

@nevrome nevrome requested a review from martynamolak September 1, 2025 14:14
@nevrome
Copy link
Member

nevrome commented Sep 1, 2025

@martynamolak offered to review the package. Thank you!

@martynamolak
Copy link
Contributor

@thanks @Tlkhi for submitting this!
Here is my comments:

janno file:

  1. Relation_To column should probably comprise Poseidon_ID of the related individuals, not the Alternative_IDs. I realise this is a bit tricky because the relation will (should) be the same regardless of the instance of a given individual's reanalysis. @nevrome , should we enforce this? (e.g. "I23568.AG" has Relation_To: "I23655" rather than to "I23655.AG")
  2. Please change "ENA: PRJEB81467" to "PRJEB81467" in the Genetic_Source_Accession_IDs field
  3. The Poseidon_ID and Group_Name have the ".SG", ".AG" and ".TW" suffixes that are, as far as I could find, not present in the original paper. I of course see the value of the suffixes but also Poseidon's policy is to keep the labels concordant with the original paper. That's something that should probably be discussed throughout the Poseidon dev team, @nevrome, @TCLamnidis, @stschiff; Anyway, I think at current stage it's probably better to leave the suffixes in; ie.: as currently is labelled in the package.
  4. Y haplogroups are provided in the hierarchichal system (ISOGG) rather than the terminal-SNP-based system (Yfull) as required/encouraged by Poseidon. The original paper provides both so they should be easy enough to replace.
  5. some individuals are merges of several libraries; some of these libraries differ in strandedness and damage removal procedure (e.g. I5124 is 4 libraries: ds.half,ds.half,ss.USER,ss.USER in original paper while in the package it's coded solely as: ds, half) - please verify all the samples and correct where necessary
  6. It would be nice to also include contamination estimates in the janno file. The original supplementary table provides HapConX and ANGSD estimates. Unfortunately they would have to be transposed from ranges to the mean+stderr format for Poseidon. Also damage rate column could be added.

All the other files in the package seem ok as far as I could see.

@Tlkhi
Copy link
Contributor Author

Tlkhi commented Sep 5, 2025

@thanks @Tlkhi for submitting this! Here is my comments:

janno file:

  1. Relation_To column should probably comprise Poseidon_ID of the related individuals, not the Alternative_IDs. I realise this is a bit tricky because the relation will (should) be the same regardless of the instance of a given individual's reanalysis. @nevrome , should we enforce this? (e.g. "I23568.AG" has Relation_To: "I23655" rather than to "I23655.AG")
  2. Please change "ENA: PRJEB81467" to "PRJEB81467" in the Genetic_Source_Accession_IDs field
  3. The Poseidon_ID and Group_Name have the ".SG", ".AG" and ".TW" suffixes that are, as far as I could find, not present in the original paper. I of course see the value of the suffixes but also Poseidon's policy is to keep the labels concordant with the original paper. That's something that should probably be discussed throughout the Poseidon dev team, @nevrome, @TCLamnidis, @stschiff; Anyway, I think at current stage it's probably better to leave the suffixes in; ie.: as currently is labelled in the package.
  4. Y haplogroups are provided in the hierarchichal system (ISOGG) rather than the terminal-SNP-based system (Yfull) as required/encouraged by Poseidon. The original paper provides both so they should be easy enough to replace.
  5. some individuals are merges of several libraries; some of these libraries differ in strandedness and damage removal procedure (e.g. I5124 is 4 libraries: ds.half,ds.half,ss.USER,ss.USER in original paper while in the package it's coded solely as: ds, half) - please verify all the samples and correct where necessary
  6. It would be nice to also include contamination estimates in the janno file. The original supplementary table provides HapConX and ANGSD estimates. Unfortunately they would have to be transposed from ranges to the mean+stderr format for Poseidon. Also damage rate column could be added.

All the other files in the package seem ok as far as I could see.

Thank you for your comments!

  1. I’m waiting for @nevrome for now - it might look a bit complicated since there is no Individual_ID column yet

  2. Fixed

  3. I think we should keep the suffixes

  4. Replaced ISOGG format with SNP-terminal based format - fixed

  5. I will mark them as mixed and note the UDG types (e.g., half; USER) in the Note column

  6. Added the Damage column, adding a Contamination column seems a bit challenging

P.S: I will update the janno in the PR after fixing all the issues

@stschiff
Copy link
Member

stschiff commented Sep 5, 2025

For what it's worth, I am OK with the suffixes... I think we have no official policy on this, and since the original authors haven't submitted the package, but @Tlkhi has, they get to decide. I believe since it's a Boston Paper, AADR-like suffixes make sense.

Stephan

@nevrome
Copy link
Member

nevrome commented Sep 8, 2025

Thanks for the review, @martynamolak, and thanks addressing it promptly, @Tlkhi.

[1]. I think we can leave it like this in anticipation of the changes planned for Poseidon v3.0.0 as discussed here poseidon-framework/poseidon-schema#74 and more concretely here poseidon-framework/poseidon-schema#109
[3]. I don't mind the suffixes as long as we have a Capture_Type column.

@Tlkhi
Copy link
Contributor Author

Tlkhi commented Sep 9, 2025

fixed the issues

@nevrome
Copy link
Member

nevrome commented Sep 14, 2025

Perfect - thanks! Sorry for the long delay. Will merge now.

@nevrome nevrome merged commit 254a0ea into poseidon-framework:master Sep 14, 2025
1 check passed
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