From 323b53fd966330bb31fe431e02e36b60f712970a Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Thu, 30 Apr 2026 16:15:14 +0100 Subject: [PATCH 01/10] add ftp link to search dump --- .../production/metadata/api/search/search.py | 107 +++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/src/ensembl/production/metadata/api/search/search.py b/src/ensembl/production/metadata/api/search/search.py index bc73ff00..c8328b70 100644 --- a/src/ensembl/production/metadata/api/search/search.py +++ b/src/ensembl/production/metadata/api/search/search.py @@ -12,6 +12,7 @@ import argparse import json import logging +import re import sys from dataclasses import dataclass from pathlib import Path @@ -138,6 +139,7 @@ class GenomeSearchDocument(BaseModel): strain: Optional[str] = None assembly_name: str accession: str + ftp: str url_name: Optional[str] = None tol_id: Optional[str] = None is_reference: bool @@ -180,6 +182,7 @@ def to_search_entry(self) -> SearchEntry: SearchField(name="scientific_name", value=self.scientific_name), SearchField(name="assembly", value=self.assembly_name), SearchField(name="assembly_accession", value=self.accession), + SearchField(name="ftp", value=self.ftp), SearchField(name="unversioned_assembly_accession", value=unversioned_accession), SearchField(name="type_value", value=self.strain or ""), SearchField(name="parlance_name", value=self.scientific_parlance_name or ""), @@ -365,6 +368,107 @@ def get_genebuild_method_display(self) -> str: return value + def get_genebuild_annotation_source(self) -> str: + """ + Get the genebuild source name used in the public FTP path. + + Raises: + MissingDatasetFieldError: If genebuild.annotation_source is not found + """ + value = self._get_dataset_attribute("genebuild", "genebuild.annotation_source") + + if value is None: + raise MissingDatasetFieldError( + f"Missing required field 'genebuild.annotation_source' for genome " + f"{self.genome.genome_uuid} in release {self.release.label}" + ) + + return value + + def get_last_geneset_update(self) -> str: + """ + Get the geneset update date used in the public FTP path. + + Raises: + MissingDatasetFieldError: If genebuild.last_geneset_update is not found + """ + value = self._get_dataset_attribute("genebuild", "genebuild.last_geneset_update") + + if value is None: + raise MissingDatasetFieldError( + f"Missing required field 'genebuild.last_geneset_update' for genome " + f"{self.genome.genome_uuid} in release {self.release.label}" + ) + + return value + + +class FTPUrlBuilder: + """Builds the public FTP URL included in the search index.""" + + BASE_URL = "https://ftp.ebi.ac.uk/pub/ensemblorganisms" + + @staticmethod + def _format_accession_path(accession: str) -> str: + """ + Convert an assembly accession into the FTP directory layout. + + Example: + GCA_000001215.4 -> GCA/000/001/215/4 + """ + match = re.match(r"^([A-Z]+)_(\d+)\.(\d+)$", accession) + if not match: + raise MissingDatasetFieldError( + f"Assembly accession '{accession}' is not in the expected format for FTP URL construction" + ) + + accession_prefix, accession_digits, accession_version = match.groups() + digit_groups = [accession_digits[i: i + 3] for i in range(0, len(accession_digits), 3)] + return "/".join([accession_prefix, *digit_groups, accession_version]) + + @staticmethod + def _format_last_geneset_update(last_geneset_update: str) -> str: + """Convert a YYYY-MM geneset date prefix to the FTP YYYY_MM directory.""" + match = re.match(r"^(\d{4}-\d{2})", last_geneset_update) + if not match: + raise MissingDatasetFieldError( + f"genebuild.last_geneset_update '{last_geneset_update}' is not in the expected YYYY-MM format" + ) + + return match.group(1).replace("-", "_") + + @classmethod + def build(cls, genome: Genome, dataset_extractor: DatasetFieldExtractor) -> str: + """ + Construct the genome-level FTP URL from metadata fields. + + The search document needs the public FTP location, but that value is not + stored directly on Genome. It is derived from the fixed EBI FTP root, + the assembly accession path, genebuild source, and geneset update date. + """ + accession = genome.assembly.accession + genebuild_source_name = dataset_extractor.get_genebuild_annotation_source() + last_geneset_update = dataset_extractor.get_last_geneset_update() + + missing_fields = [] + if not accession: + missing_fields.append("assembly.accession") + if not genebuild_source_name: + missing_fields.append("genebuild.annotation_source") + if not last_geneset_update: + missing_fields.append("genebuild.last_geneset_update") + + if missing_fields: + raise MissingDatasetFieldError( + f"Missing required field(s) for FTP URL construction for genome " + f"{genome.genome_uuid}: {', '.join(missing_fields)}" + ) + + accession_path = cls._format_accession_path(accession) + source_path = genebuild_source_name.lower() + update_path = cls._format_last_geneset_update(last_geneset_update) + return f"{cls.BASE_URL}/{accession_path}/{source_path}/{update_path}/" + # ============================================================================ # RELEASE SELECTION HELPER @@ -649,7 +753,7 @@ def _get_genome_ids_to_process(self, session: Session) -> List[int]: .distinct() .all() ) - return [gid[0] for gid in genome_ids] + return [gid[0] for gid in genome_ids][:1] # TODO remove limit after testing def _get_genomes_batch(self, session: Session, genome_ids: List[int]) -> List[Genome]: """ @@ -827,6 +931,7 @@ def create_search_document( # Extract dataset fields - will raise exceptions if required fields are missing dataset_extractor = DatasetFieldExtractor(metadata_session, genome, release) + doc_data["ftp"] = FTPUrlBuilder.build(genome, dataset_extractor) doc_data.update( { "contig_n50": dataset_extractor.get_contig_n50(), From 0ca9ace0e025556529d839f4a1ee23bda3527c0f Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Thu, 30 Apr 2026 16:33:44 +0100 Subject: [PATCH 02/10] fix broken tests --- src/ensembl/production/metadata/api/search/search.py | 2 +- src/tests/test_search.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ensembl/production/metadata/api/search/search.py b/src/ensembl/production/metadata/api/search/search.py index c8328b70..02281678 100644 --- a/src/ensembl/production/metadata/api/search/search.py +++ b/src/ensembl/production/metadata/api/search/search.py @@ -753,7 +753,7 @@ def _get_genome_ids_to_process(self, session: Session) -> List[int]: .distinct() .all() ) - return [gid[0] for gid in genome_ids][:1] # TODO remove limit after testing + return [gid[0] for gid in genome_ids] def _get_genomes_batch(self, session: Session, genome_ids: List[int]) -> List[Genome]: """ diff --git a/src/tests/test_search.py b/src/tests/test_search.py index 70da792a..b7d9897c 100644 --- a/src/tests/test_search.py +++ b/src/tests/test_search.py @@ -181,6 +181,7 @@ def test_valid_document_creation(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -219,6 +220,7 @@ def test_document_with_optional_fields(self, test_dbs): strain="GRCh38", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", url_name="homo_sapiens", tol_id="mHomSap1", is_reference=True, @@ -291,6 +293,7 @@ def test_document_model_dump(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -322,6 +325,7 @@ def test_rank_injection(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -345,6 +349,7 @@ def test_rank_injection(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, From e84ddda3957a7cf6477c8b75ec3e4d80d5167c60 Mon Sep 17 00:00:00 2001 From: danielp Date: Tue, 5 May 2026 14:11:11 +0100 Subject: [PATCH 03/10] Updated get_ftp_paths for the new structure and fixed a couple bugs --- .../metadata/api/adaptors/genome.py | 163 ++++++++++++++---- .../metadata/api/factories/utils.py | 34 ++++ src/tests/test_api.py | 20 +-- 3 files changed, 177 insertions(+), 40 deletions(-) diff --git a/src/ensembl/production/metadata/api/adaptors/genome.py b/src/ensembl/production/metadata/api/adaptors/genome.py index d2f54755..18d13231 100644 --- a/src/ensembl/production/metadata/api/adaptors/genome.py +++ b/src/ensembl/production/metadata/api/adaptors/genome.py @@ -25,6 +25,7 @@ from ensembl.production.metadata.api.adaptors.base import BaseAdaptor, check_parameter, cfg from ensembl.production.metadata.api.exceptions import TypeNotFoundException +from ensembl.production.metadata.api.factories.utils import format_accession_path from ensembl.production.metadata.api.models import * logger = logging.getLogger(__name__) @@ -344,7 +345,7 @@ def fetch_genomes( or_( or_( EnsemblRelease.release_date > Er_2.release_date, - Er_2.release_date is None + Er_2.release_date.is_(None) ), and_( EnsemblRelease.release_date == Er_2.release_date, @@ -534,7 +535,7 @@ def fetch_genome_by_release_version(self, release_version, status="Released"): .join(EnsemblRelease, EnsemblRelease.release_id == GenomeRelease.release_id) \ .join(EnsemblSite, EnsemblRelease.site_id == EnsemblSite.site_id & EnsemblSite.site_id == cfg.ensembl_site_id) - if status == GenomeStatus.RELEASED or GenomeStatus.CURRENT: + if status in (GenomeStatus.RELEASED, GenomeStatus.CURRENT): genome_query = genome_query.filter(EnsemblRelease.status == ReleaseStatus.RELEASED) if release_version is not None and release_version > 0: genome_query = genome_query.where(EnsemblRelease.version == release_version) @@ -991,7 +992,7 @@ def fetch_genome_group_members_detailed( Returns: List of tuples (Genome, GenomeGroupMember) with full membership details. """ - status = GenomeStatus[status.upper()] + status = GenomeStatus[release_status.upper()] member_select = select(Genome, GenomeGroupMember).join( GenomeGroupMember, Genome.genome_id == GenomeGroupMember.genome_id ).join( @@ -1027,14 +1028,45 @@ def fetch_genome_group_members_detailed( session.expire_on_commit = False return session.execute(member_select).all() - def get_public_path(self, genome_uuid, dataset_type='all'): + def get_public_path(self, genome_uuid, dataset_type='all', release=None): + """ + Retrieve public file paths for genomic datasets. + + Args: + genome_uuid (str): Unique identifier for the genome + dataset_type (str): Type of dataset ('genebuild', 'assembly', 'homologies', 'variation', or 'all') + release (str, optional): Specific Ensembl release label. If None, uses current release. + + Returns: + list: List of dictionaries containing dataset_type and path information + + Raises: + ValueError: If genome_uuid or release not found, or required metadata missing + TypeNotFoundException: If requested dataset_type is not available + """ paths = [] scientific_name = None accession = None genebuild_source_name = None last_geneset_update = None + with self.metadata_db.session_scope() as session: - # Single combined query to get all required data + # === VALIDATION SECTION === + genome_exists = session.execute( + select(Genome.genome_uuid).where(Genome.genome_uuid == genome_uuid) + ).first() + if not genome_exists: + raise ValueError(f"Genome with UUID {genome_uuid} not found") + + if release is not None: + release_exists = session.execute( + select(EnsemblRelease.label).where(EnsemblRelease.label == release) + ).first() + if not release_exists: + raise ValueError(f"Ensembl release with label '{release}' not found") + + # === METADATA RETRIEVAL === + # Get core genome metadata: organism name, assembly accession, and genebuild info query = select( Organism.scientific_name, Assembly.accession, @@ -1064,57 +1096,130 @@ def get_public_path(self, genome_uuid, dataset_type='all'): else: scientific_name = accession = genebuild_source_name = last_geneset_update = None - # Query for which of the 5 supported dataset types exist for this genome - supported_types = ['genebuild', 'assembly', 'homologies', 'regulatory_features', 'variation'] + # === DATASET TYPE DISCOVERY === + supported_types = ['genebuild', 'assembly', 'homologies', 'variation'] unique_dataset_types_query = select(DatasetType.name).distinct().join( Dataset ).join(GenomeDataset).join(Genome).where( Genome.genome_uuid == genome_uuid, - DatasetType.name.in_(supported_types) + DatasetType.name.in_(supported_types), + Dataset.status == DatasetStatus.RELEASED ) unique_dataset_types = session.execute(unique_dataset_types_query).scalars().all() + variation_release = None + homology_release = None + + # === RELEASE HANDLING === + if release is None: + if 'variation' in unique_dataset_types: + variation_release = session.execute( + select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( + Genome).where( + Genome.genome_uuid == genome_uuid, + DatasetType.name == 'variation', + Dataset.status == DatasetStatus.RELEASED, + GenomeDataset.is_current == True, + EnsemblRelease.release_type == 'partial' + ) + ).scalar_one_or_none() + + if 'homologies' in unique_dataset_types: + homology_release = session.execute( + select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( + Genome).where( + Genome.genome_uuid == genome_uuid, + DatasetType.name == 'homologies', + Dataset.status == DatasetStatus.RELEASED, + GenomeDataset.is_current == True, + EnsemblRelease.release_type == 'partial' + ) + ).scalar_one_or_none() + else: + if 'variation' in unique_dataset_types: + variation_release = session.execute( + select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( + Genome).where( + Genome.genome_uuid == genome_uuid, + DatasetType.name == 'variation', + Dataset.status == DatasetStatus.RELEASED, + EnsemblRelease.release_type == 'partial', + EnsemblRelease.release_id <= ( + select(EnsemblRelease.release_id).where( + EnsemblRelease.label == release).scalar_subquery() + ) + ).order_by(EnsemblRelease.release_id.desc()) + ).scalar_one_or_none() + + if variation_release is None: + unique_dataset_types = [t for t in unique_dataset_types if t != 'variation'] + + if 'homologies' in unique_dataset_types: + homology_release = session.execute( + select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( + Genome).where( + Genome.genome_uuid == genome_uuid, + DatasetType.name == 'homologies', + Dataset.status == DatasetStatus.RELEASED, + EnsemblRelease.release_type == 'partial', + EnsemblRelease.release_id <= ( + select(EnsemblRelease.release_id).where( + EnsemblRelease.label == release).scalar_subquery() + ) + ).order_by(EnsemblRelease.release_id.desc()) + ).scalar_one_or_none() + + if homology_release is None: + unique_dataset_types = [t for t in unique_dataset_types if t != 'homologies'] + + # === DATA VALIDATION === + if 'genebuild' not in unique_dataset_types or 'assembly' not in unique_dataset_types: + raise ValueError( + f"Missing genebuild or assembly dataset types. Something is seriously wrong with {genome_uuid}") + if scientific_name is None or accession is None or genebuild_source_name is None or last_geneset_update is None: raise ValueError("Required metadata fields are missing. Please check the database entries.") - unique_dataset_types = ['regulation' if t == 'regulatory_features' else t for t in unique_dataset_types] - if dataset_type == 'regulatory_features': - dataset_type = 'regulation' - match = re.match(r'^(\d{4}-\d{2})', last_geneset_update) # Match format YYYY-MM + + # === PATH CONSTRUCTION === + match = re.match(r'^(\d{4}-\d{2})', last_geneset_update) + if not match: + raise ValueError(f"Invalid last_geneset_update format: {last_geneset_update}") last_geneset_update = match.group(1).replace('-', '_') - scientific_name = re.sub(r'[^a-zA-Z0-9]+', ' ', scientific_name) - scientific_name = scientific_name.replace(' ', '_') - scientific_name = re.sub(r'^_+|_+$', '', scientific_name) + genebuild_source_name = genebuild_source_name.lower() - base_path = f"{scientific_name}/{accession}" - common_path = f"{base_path}/{genebuild_source_name}" + + base_path = format_accession_path(accession) + common_path = f"{base_path}/{genebuild_source_name}/{last_geneset_update}" + + if 'homologies' in unique_dataset_types and homology_release: + homology_release = homology_release.replace('-', '_') + if 'variation' in unique_dataset_types and variation_release: + variation_release = variation_release.replace('-', '_') path_templates = { - 'genebuild': f"{common_path}/geneset/{last_geneset_update}", - 'assembly': f"{base_path}/genome", - 'homologies': f"{common_path}/homology/{last_geneset_update}", - 'regulation': f"{common_path}/regulation", - 'variation': f"{common_path}/variation/{last_geneset_update}", + 'genebuild': f"{common_path}/geneset", + 'assembly': f"{common_path}/genome", + 'homologies': f"{common_path}/homology/{homology_release}", + 'variation': f"{common_path}/variation/{variation_release}", } - # Check for invalid dataset type early + # === REQUEST VALIDATION === if dataset_type not in unique_dataset_types and dataset_type != 'all': raise TypeNotFoundException(f"Dataset Type : {dataset_type} not found in metadata.") - # If 'all', add paths for all unique dataset types + # === PATH GENERATION === if dataset_type == 'all': - for t in unique_dataset_types: + for dataset_type_name in unique_dataset_types: paths.append({ - "dataset_type": t, - "path": path_templates[t] + "dataset_type": dataset_type_name, + "path": path_templates[dataset_type_name] }) elif dataset_type in path_templates: - # Add path for the specific dataset type paths.append({ "dataset_type": dataset_type, "path": path_templates[dataset_type] }) else: - # If the code reaches here, it means there is a logic error raise TypeNotFoundException(f"Dataset Type : {dataset_type} has no associated path.") return paths diff --git a/src/ensembl/production/metadata/api/factories/utils.py b/src/ensembl/production/metadata/api/factories/utils.py index 005e3d21..1d6d9cad 100644 --- a/src/ensembl/production/metadata/api/factories/utils.py +++ b/src/ensembl/production/metadata/api/factories/utils.py @@ -9,9 +9,12 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import os +import re from ensembl.production.metadata.api.models import Genome, Assembly + def get_genome_sets_by_assembly_and_provider(session): """ Retrieves only those sets where multiple genome_uuids share the same assembly_uuid and genebuild.provider. @@ -46,3 +49,34 @@ def get_genome_sets_by_assembly_and_provider(session): genome_sets_with_multiple = {key: genomes for key, genomes in genome_sets.items() if len(genomes) > 1} return genome_sets_with_multiple + + +def format_accession_path(accession: str) -> str: + """ + Converts assembly accession (e.g., 'GCF_043381705.1') to a structured path format + (e.g., 'GCF/043/381/705/1'). + + Parameters: + accession (str): The accession string to format. + + Returns: + str: Formatted path-like string. + + Raises: + ValueError: If the accession format is invalid. + """ + pattern = r'^(GCF|GCA)_(\d+)\.(\d+)$' + match = re.match(pattern, accession) + + if not match: + raise ValueError(f"Invalid accession format: '{accession}'. Expected format like '(GCF|GCA)_#########.#'.") + + prefix, number, version = match.groups() + + if len(number) > 9: + raise ValueError(f"Unexpected number length in accession: '{number}'. Expected up to 9 digits.") + + chunks = [number[i:i + 3] for i in range(0, 9, 3)] + + # Ensure we have exactly 3 chunks, filling with '000' if necessary + return os.path.join(prefix, *chunks, version) diff --git a/src/tests/test_api.py b/src/tests/test_api.py index 38bb4c9b..32c53872 100644 --- a/src/tests/test_api.py +++ b/src/tests/test_api.py @@ -33,13 +33,13 @@ def test_get_public_path(self, test_dbs): assert len(paths) == 4 # assert all("/genebuild/" in path for path in paths) path = genome_adapter.get_public_path(genome_uuid, dataset_type='genebuild') - assert path[0]['path'] == 'Saccharomyces_cerevisiae_S288c/GCA_000146045.2/community/geneset/2018_10' + assert path[0]['path'] == 'GCA/000/146/045/2/community/2018_10/geneset' path = genome_adapter.get_public_path(genome_uuid, dataset_type='assembly') - assert path[0]['path'] == 'Saccharomyces_cerevisiae_S288c/GCA_000146045.2/genome' + assert path[0]['path'] == 'GCA/000/146/045/2/community/2018_10/genome' path = genome_adapter.get_public_path(genome_uuid, dataset_type='variation') - assert path[0]['path'] == 'Saccharomyces_cerevisiae_S288c/GCA_000146045.2/community/variation/2018_10' + assert path[0]['path'] == 'GCA/000/146/045/2/community/2018_10/variation/2023_06_15' path = genome_adapter.get_public_path(genome_uuid, dataset_type='homologies') - assert path[0]['path'] == 'Saccharomyces_cerevisiae_S288c/GCA_000146045.2/community/homology/2018_10' + assert path[0]['path'] == 'GCA/000/146/045/2/community/2018_10/homology/2023_06_15' with pytest.raises(TypeNotFoundException): genome_adapter.get_public_path(genome_uuid, dataset_type='regulatory_features') # assert path[0]['path'] == 'Saccharomyces_cerevisiae_S288c/GCA_000146045.2/ensembl/regulation' @@ -49,15 +49,13 @@ def test_default_public_path(self, test_dbs): genome_uuid = 'a7335667-93e7-11ec-a39d-005056b38ce3' # Homo sapien GRCH38 paths = genome_adapter.get_public_path(genome_uuid, dataset_type='all') - assert len(paths) == 5 + assert len(paths) == 4 # assert all("/genebuild/" in path for path in paths) path = genome_adapter.get_public_path(genome_uuid, dataset_type='genebuild') - assert path[0]['path'] == 'Homo_sapiens/GCA_000001405.29/ensembl/geneset/2023_03' + assert path[0]['path'] == 'GCA/000/001/405/29/ensembl/2023_03/geneset' path = genome_adapter.get_public_path(genome_uuid, dataset_type='assembly') - assert path[0]['path'] == 'Homo_sapiens/GCA_000001405.29/genome' + assert path[0]['path'] == 'GCA/000/001/405/29/ensembl/2023_03/genome' path = genome_adapter.get_public_path(genome_uuid, dataset_type='variation') - assert path[0]['path'] == 'Homo_sapiens/GCA_000001405.29/ensembl/variation/2023_03' + assert path[0]['path'] == 'GCA/000/001/405/29/ensembl/2023_03/variation/2023_06_15' path = genome_adapter.get_public_path(genome_uuid, dataset_type='homologies') - assert path[0]['path'] == 'Homo_sapiens/GCA_000001405.29/ensembl/homology/2023_03' - path = genome_adapter.get_public_path(genome_uuid, dataset_type='regulatory_features') - assert path[0]['path'] == 'Homo_sapiens/GCA_000001405.29/ensembl/regulation' + assert path[0]['path'] == 'GCA/000/001/405/29/ensembl/2023_03/homology/2023_06_15' From 944d1d0335dda0038ff540d949903a4ba6bef1f8 Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Thu, 7 May 2026 10:02:19 +0100 Subject: [PATCH 04/10] remve FTPUrlBuilder and use get_public_path from the adaptor --- .../production/metadata/api/search/search.py | 136 +++++------------- 1 file changed, 32 insertions(+), 104 deletions(-) diff --git a/src/ensembl/production/metadata/api/search/search.py b/src/ensembl/production/metadata/api/search/search.py index 02281678..fe4f3c97 100644 --- a/src/ensembl/production/metadata/api/search/search.py +++ b/src/ensembl/production/metadata/api/search/search.py @@ -12,7 +12,6 @@ import argparse import json import logging -import re import sys from dataclasses import dataclass from pathlib import Path @@ -23,6 +22,7 @@ from sqlalchemy.exc import NoResultFound from sqlalchemy.orm import Session, joinedload +from ensembl.production.metadata.api.adaptors.genome import GenomeAdaptor from ensembl.production.metadata.api.models import ( Genome, Dataset, @@ -368,107 +368,6 @@ def get_genebuild_method_display(self) -> str: return value - def get_genebuild_annotation_source(self) -> str: - """ - Get the genebuild source name used in the public FTP path. - - Raises: - MissingDatasetFieldError: If genebuild.annotation_source is not found - """ - value = self._get_dataset_attribute("genebuild", "genebuild.annotation_source") - - if value is None: - raise MissingDatasetFieldError( - f"Missing required field 'genebuild.annotation_source' for genome " - f"{self.genome.genome_uuid} in release {self.release.label}" - ) - - return value - - def get_last_geneset_update(self) -> str: - """ - Get the geneset update date used in the public FTP path. - - Raises: - MissingDatasetFieldError: If genebuild.last_geneset_update is not found - """ - value = self._get_dataset_attribute("genebuild", "genebuild.last_geneset_update") - - if value is None: - raise MissingDatasetFieldError( - f"Missing required field 'genebuild.last_geneset_update' for genome " - f"{self.genome.genome_uuid} in release {self.release.label}" - ) - - return value - - -class FTPUrlBuilder: - """Builds the public FTP URL included in the search index.""" - - BASE_URL = "https://ftp.ebi.ac.uk/pub/ensemblorganisms" - - @staticmethod - def _format_accession_path(accession: str) -> str: - """ - Convert an assembly accession into the FTP directory layout. - - Example: - GCA_000001215.4 -> GCA/000/001/215/4 - """ - match = re.match(r"^([A-Z]+)_(\d+)\.(\d+)$", accession) - if not match: - raise MissingDatasetFieldError( - f"Assembly accession '{accession}' is not in the expected format for FTP URL construction" - ) - - accession_prefix, accession_digits, accession_version = match.groups() - digit_groups = [accession_digits[i: i + 3] for i in range(0, len(accession_digits), 3)] - return "/".join([accession_prefix, *digit_groups, accession_version]) - - @staticmethod - def _format_last_geneset_update(last_geneset_update: str) -> str: - """Convert a YYYY-MM geneset date prefix to the FTP YYYY_MM directory.""" - match = re.match(r"^(\d{4}-\d{2})", last_geneset_update) - if not match: - raise MissingDatasetFieldError( - f"genebuild.last_geneset_update '{last_geneset_update}' is not in the expected YYYY-MM format" - ) - - return match.group(1).replace("-", "_") - - @classmethod - def build(cls, genome: Genome, dataset_extractor: DatasetFieldExtractor) -> str: - """ - Construct the genome-level FTP URL from metadata fields. - - The search document needs the public FTP location, but that value is not - stored directly on Genome. It is derived from the fixed EBI FTP root, - the assembly accession path, genebuild source, and geneset update date. - """ - accession = genome.assembly.accession - genebuild_source_name = dataset_extractor.get_genebuild_annotation_source() - last_geneset_update = dataset_extractor.get_last_geneset_update() - - missing_fields = [] - if not accession: - missing_fields.append("assembly.accession") - if not genebuild_source_name: - missing_fields.append("genebuild.annotation_source") - if not last_geneset_update: - missing_fields.append("genebuild.last_geneset_update") - - if missing_fields: - raise MissingDatasetFieldError( - f"Missing required field(s) for FTP URL construction for genome " - f"{genome.genome_uuid}: {', '.join(missing_fields)}" - ) - - accession_path = cls._format_accession_path(accession) - source_path = genebuild_source_name.lower() - update_path = cls._format_last_geneset_update(last_geneset_update) - return f"{cls.BASE_URL}/{accession_path}/{source_path}/{update_path}/" - # ============================================================================ # RELEASE SELECTION HELPER @@ -733,9 +632,12 @@ def validate_unique_url_names(entries: List[SearchEntry]) -> None: class GenomeSearchIndexer: """Service for generating genome search documents""" + FTP_BASE_URL = "https://ftp.ebi.ac.uk/pub/ensemblorganisms" + def __init__(self, metadata_uri: str, taxonomy_uri: str, batch_size: int = 500): self.metadata_db = DBConnection(metadata_uri) self.taxonomy_db = DBConnection(taxonomy_uri) + self.genome_adaptor = GenomeAdaptor(self.metadata_db, self.taxonomy_db) self.release_selector = ReleaseSelector() self.deduplicator = EntryDeduplicator() self.batch_size = batch_size @@ -753,7 +655,7 @@ def _get_genome_ids_to_process(self, session: Session) -> List[int]: .distinct() .all() ) - return [gid[0] for gid in genome_ids] + return [gid[0] for gid in genome_ids][:10] def _get_genomes_batch(self, session: Session, genome_ids: List[int]) -> List[Genome]: """ @@ -845,6 +747,32 @@ def _extract_direct_fields(self, genome: Genome) -> dict: "rank": genome.organism.rank or 0, } + def _get_ftp_path(self, genome: Genome, release: EnsemblRelease) -> str: + """ + Build the search FTP URL using GenomeAdaptor.get_public_path as the source of truth. + + get_public_path returns dataset-specific relative paths. The search index stores + the genome-level FTP directory, so use the genebuild path and remove its final + dataset directory. + """ + paths = self.genome_adaptor.get_public_path( + genome_uuid=genome.genome_uuid, + dataset_type="genebuild", + release=release.label if release else None, + ) + + genebuild_path = next( + (path_info["path"] for path_info in paths if path_info["dataset_type"] == "genebuild"), + None, + ) + if not genebuild_path: + raise MissingDatasetFieldError( + f"Could not construct FTP path for genome {genome.genome_uuid}: genebuild path not found" + ) + + genome_path = genebuild_path.removesuffix("/geneset") + return f"{self.FTP_BASE_URL}/{genome_path}/" + def _get_taxonomy_lineage( self, taxonomy_session: Session, taxonomy_id: int ) -> Tuple[List[int], List[str]]: @@ -931,7 +859,6 @@ def create_search_document( # Extract dataset fields - will raise exceptions if required fields are missing dataset_extractor = DatasetFieldExtractor(metadata_session, genome, release) - doc_data["ftp"] = FTPUrlBuilder.build(genome, dataset_extractor) doc_data.update( { "contig_n50": dataset_extractor.get_contig_n50(), @@ -942,6 +869,7 @@ def create_search_document( "genebuild_method_display": dataset_extractor.get_genebuild_method_display(), } ) + doc_data["ftp"] = self._get_ftp_path(genome, release) lineage_taxids, lineage_names = self._get_taxonomy_lineage( taxonomy_session, genome.organism.taxonomy_id From 8a9dc72de77626e2f67cff333cbd9afdc663e075 Mon Sep 17 00:00:00 2001 From: danielp Date: Fri, 8 May 2026 13:44:20 +0100 Subject: [PATCH 05/10] Full rewrite of Json exporter for ftp for new structure and associated unit tests --- .../metadata/api/exports/ftp_index.py | 409 ++++++++++-------- src/tests/test_exports_json.py | 259 ++++++----- 2 files changed, 393 insertions(+), 275 deletions(-) diff --git a/src/ensembl/production/metadata/api/exports/ftp_index.py b/src/ensembl/production/metadata/api/exports/ftp_index.py index 9cd6bf2b..2f1eb984 100644 --- a/src/ensembl/production/metadata/api/exports/ftp_index.py +++ b/src/ensembl/production/metadata/api/exports/ftp_index.py @@ -29,15 +29,49 @@ from ensembl.production.metadata.api.models.release import EnsemblRelease, ReleaseStatus +def format_accession_path(accession): + """ + Convert an assembly accession to its FTP directory path format. + + E.g. GCA_000001405.29 -> GCA/000/001/405/29 + + The numeric portion (zero-padded to 9 digits) is split into three 3-digit + chunks, and the version suffix becomes the final path component. + """ + match = re.match(r'^(GC[AF])_(\d+)\.(\d+)$', accession) + if not match: + raise ValueError(f"Invalid accession format: {accession}") + prefix = match.group(1) + digits = match.group(2).zfill(9) # ensure 9 digits for the 3-chunk split + version = match.group(3) + chunks = [digits[i:i + 3] for i in range(0, len(digits), 3)] + return f"{prefix}/{'/'.join(chunks)}/{version}" + + class FTPMetadataExporter: """ Independent class for generating FTP metadata JSON structure. - Builds hierarchical data organized by species -> assemblies -> providers -> releases. + Builds hierarchical data organised by species -> assemblies -> providers -> releases. + + New FTP path structure + ---------------------- + {accession_path}/{annotation_source}/{geneset_date}/ + geneset/ <- genebuild files + genome/ <- assembly / repeat-masked genome files + homology/{date}/ <- homology files (partial release date) + variation/{date}/ <- variation files (partial release date, if present) + + Index files (.fai, .gzi, .csi) and checksum files (md5sum.txt) are + intentionally excluded from the JSON listing. """ def __init__(self, metadata_uri): self.metadata_db = DBConnection(metadata_uri) + # ------------------------------------------------------------------ + # Public interface + # ------------------------------------------------------------------ + def export_to_json(self, output_file=None): """ Export FTP metadata to JSON format. @@ -75,9 +109,14 @@ def build_ftp_metadata_json(self): return metadata_structure + # ------------------------------------------------------------------ + # Data loading + # ------------------------------------------------------------------ + def _load_all_genome_data(self, session): - """Load all genome data in bulk queries to minimize database round trips.""" + """Load all genome data in bulk queries to minimise database round trips.""" + # --- Genomes with at least one released dataset or released genome-release --- genomes_query = select(Genome).options( selectinload(Genome.organism), selectinload(Genome.assembly), @@ -93,6 +132,7 @@ def _load_all_genome_data(self, session): if not genome_uuids: return {} + # --- Released datasets for all genomes --- datasets_query = select( Genome.genome_uuid, Dataset, @@ -110,12 +150,13 @@ def _load_all_genome_data(self, session): DatasetSource, Dataset.dataset_source_id == DatasetSource.dataset_source_id ).where( Genome.genome_uuid.in_(genome_uuids), - (Dataset.status == DatasetStatus.RELEASED) + Dataset.status == DatasetStatus.RELEASED ) dataset_results = session.execute(datasets_query).all() dataset_ids = [r.Dataset.dataset_id for r in dataset_results] + # --- Dataset attributes --- attributes_query = select( DatasetAttribute.dataset_id, Attribute.name.label('attribute_name'), @@ -128,6 +169,7 @@ def _load_all_genome_data(self, session): attribute_results = session.execute(attributes_query).all() + # --- Genebuild metadata (annotation source + geneset date) --- genebuild_query = select( Genome.genome_uuid, Organism.scientific_name, @@ -168,6 +210,40 @@ def _load_all_genome_data(self, session): genebuild_results = session.execute(genebuild_query).all() + # --- Partial release labels for homologies and variation --- + # These provide the sub-directory date component under homology/ and variation/. + # Mirrors the logic in get_public_path: partial releases, is_current=True, released status. + partial_releases_query = select( + Genome.genome_uuid, + DatasetType.name.label('dataset_type_name'), + EnsemblRelease.label + ).select_from( + EnsemblRelease + ).join( + GenomeDataset + ).join( + Dataset, GenomeDataset.dataset_id == Dataset.dataset_id + ).join( + DatasetType, Dataset.dataset_type_id == DatasetType.dataset_type_id + ).join( + Genome, GenomeDataset.genome_id == Genome.genome_id + ).where( + Genome.genome_uuid.in_(genome_uuids), + DatasetType.name.in_(['homologies', 'variation']), + Dataset.status == DatasetStatus.RELEASED, + GenomeDataset.is_current == True, + EnsemblRelease.release_type == 'partial' + ) + + partial_release_results = session.execute(partial_releases_query).all() + + # --- Assemble per-genome partial release lookup --- + # genome_uuid -> { 'homologies': label, 'variation': label } + partial_releases_by_genome = defaultdict(dict) + for result in partial_release_results: + partial_releases_by_genome[result.genome_uuid][result.dataset_type_name] = result.label + + # --- Build genome_data dict --- genome_data = {} for genome in genomes: @@ -197,15 +273,24 @@ def _load_all_genome_data(self, session): for result in genebuild_results: if result.genome_uuid in genome_data: + genome_partial = partial_releases_by_genome.get(result.genome_uuid, {}) genome_data[result.genome_uuid]['genebuild_metadata'] = { 'scientific_name': result.scientific_name, 'accession': result.accession, 'genebuild_source_name': result.genebuild_source_name, - 'last_geneset_update': result.last_geneset_update + 'last_geneset_update': result.last_geneset_update, + # Partial release labels used as sub-directory names for homology/variation. + # None means the dataset type has no released partial release and will be omitted. + 'homology_release': genome_partial.get('homologies'), + 'variation_release': genome_partial.get('variation'), } return genome_data + # ------------------------------------------------------------------ + # Processing + # ------------------------------------------------------------------ + def _process_genome_data(self, data, metadata_structure): """Process a single genome's data using preloaded information.""" @@ -247,11 +332,6 @@ def _process_genome_datasets_bulk(self, genome_data, assembly_data): datasets = genome_data['datasets'] genebuild_metadata = genome_data['genebuild_metadata'] - genome_is_released = any( - gr.ensembl_release and gr.ensembl_release.status == ReleaseStatus.RELEASED - for gr in genome.genome_releases - ) - if genebuild_metadata and genebuild_metadata.get('last_geneset_update'): genebuild_release_info = self._extract_genebuild_release_info(genebuild_metadata) else: @@ -262,9 +342,7 @@ def _process_genome_datasets_bulk(self, genome_data, assembly_data): seen_homology_paths = set() for dataset_info in datasets: - dataset = dataset_info['dataset'] dataset_type = dataset_info['dataset_type_name'] - provider_for_path = self._extract_provider_from_path(genebuild_metadata) if dataset_type == 'assembly': @@ -289,26 +367,22 @@ def _process_genome_datasets_bulk(self, genome_data, assembly_data): if homology_path in seen_homology_paths: continue seen_homology_paths.add(homology_path) - except: + except Exception: pass provider_datasets[provider_for_path][release_key].append(dataset_info) + # --- Assembly / genome files --- if assembly_dataset_info and genebuild_metadata: try: assembly_paths = self._get_public_paths_bulk(genebuild_metadata, datasets, 'assembly') if assembly_paths: - assembly_path = assembly_paths[0]["path"] - file_paths = self._get_dataset_file_paths( - assembly_path, 'assembly', genome, assembly_data - ) - - assembly_data["assembly"] = { - "files": file_paths - } + file_paths = self._get_dataset_file_paths(assembly_paths[0]["path"], 'assembly') + assembly_data["assembly"] = {"files": file_paths} except Exception as e: print(f"Error generating assembly paths for genome {genome.genome_uuid}: {e}") + # --- All other dataset types --- for provider, releases in provider_datasets.items(): for release_key, dataset_list in releases.items(): release_data = assembly_data["genebuild_providers"][provider][release_key] @@ -325,12 +399,9 @@ def _process_genome_datasets_bulk(self, genome_data, assembly_data): if self._has_released_dataset_bulk(datasets, dataset_type): file_paths = self._get_dataset_file_paths( - path_info["path"], dataset_type, genome, assembly_data + path_info["path"], dataset_type ) - - release_data["paths"][dataset_type] = { - "files": file_paths - } + release_data["paths"][dataset_type] = {"files": file_paths} except Exception as e: error_msg = f"Error generating paths for genome {genome.genome_uuid}: {e}" @@ -338,114 +409,203 @@ def _process_genome_datasets_bulk(self, genome_data, assembly_data): error_msg += f" (Provider: {provider}, Release: {release_key})" print(error_msg) + # ------------------------------------------------------------------ + # Path construction + # ------------------------------------------------------------------ + def _get_public_paths_bulk(self, genebuild_metadata, datasets, dataset_type='all'): - """Generate public FTP paths using preloaded metadata.""" + """ + Generate public FTP paths using preloaded metadata. + New structure: + {accession_path}/{annotation_source}/{geneset_date}/geneset/ + {accession_path}/{annotation_source}/{geneset_date}/genome/ + {accession_path}/{annotation_source}/{geneset_date}/homology/{partial_release}/ + {accession_path}/{annotation_source}/{geneset_date}/variation/{partial_release}/ + """ if not genebuild_metadata: return [] - scientific_name = genebuild_metadata['scientific_name'] accession = genebuild_metadata['accession'] genebuild_source_name = genebuild_metadata['genebuild_source_name'] last_geneset_update = genebuild_metadata['last_geneset_update'] + homology_release = genebuild_metadata.get('homology_release') + variation_release = genebuild_metadata.get('variation_release') + # Validate required fields missing_fields = [] - if not scientific_name: - missing_fields.append("scientific_name") if not accession: missing_fields.append("assembly.accession") if not genebuild_source_name: missing_fields.append("genebuild.annotation_source") if not last_geneset_update: missing_fields.append("genebuild.last_geneset_update") - if missing_fields: raise ValueError( - f"Required metadata fields are missing: {', '.join(missing_fields)}. Please check the database entries.") + f"Required metadata fields are missing: {', '.join(missing_fields)}. " + "Please check the database entries." + ) + + # Normalise geneset date to YYYY_MM + match = re.match(r'^(\d{4}-\d{2})', last_geneset_update) + if not match: + raise ValueError(f"Invalid last_geneset_update format: {last_geneset_update}") + geneset_date = match.group(1).replace('-', '_') + + # Normalise partial release labels to YYYY_MM_DD (or whatever format they carry) + if homology_release: + homology_release = homology_release.replace('-', '_') + if variation_release: + variation_release = variation_release.replace('-', '_') + # Collect the dataset types actually present for this genome unique_dataset_types = list(set([ 'regulation' if d['dataset_type_name'] == 'regulatory_features' else d['dataset_type_name'] for d in datasets ])) + # Drop homologies / variation if there is no partial release date available + if not homology_release and 'homologies' in unique_dataset_types: + unique_dataset_types = [t for t in unique_dataset_types if t != 'homologies'] + if not variation_release and 'variation' in unique_dataset_types: + unique_dataset_types = [t for t in unique_dataset_types if t != 'variation'] + + # Normalise incoming dataset_type alias if dataset_type == 'regulatory_features': dataset_type = 'regulation' - match = re.match(r'^(\d{4}-\d{2})', last_geneset_update) - if match: - last_geneset_update = match.group(1).replace('-', '_') - - scientific_name = self._normalize_species_name(scientific_name) + # Build paths + accession_path = format_accession_path(accession) genebuild_source_name = genebuild_source_name.lower() - base_path = f"{scientific_name}/{accession}" - common_path = f"{base_path}/{genebuild_source_name}" + common_path = f"{accession_path}/{genebuild_source_name}/{geneset_date}" path_templates = { - 'genebuild': f"{common_path}/geneset/{last_geneset_update}", - 'assembly': f"{base_path}/genome", - 'homologies': f"{common_path}/homology/{last_geneset_update}", - 'regulation': f"{common_path}/regulation", - 'variation': f"{common_path}/variation/{last_geneset_update}", + 'genebuild': f"{common_path}/geneset", + 'assembly': f"{common_path}/genome", + 'homologies': f"{common_path}/homology/{homology_release}", + 'variation': f"{common_path}/variation/{variation_release}", } - paths = [] - if dataset_type not in unique_dataset_types and dataset_type != 'all': raise TypeNotFoundException(f"Dataset Type : {dataset_type} not found in metadata.") + paths = [] if dataset_type == 'all': for t in unique_dataset_types: if t in path_templates: - paths.append({ - "dataset_type": t, - "path": path_templates[t] - }) + paths.append({"dataset_type": t, "path": path_templates[t]}) elif dataset_type in path_templates: - paths.append({ - "dataset_type": dataset_type, - "path": path_templates[dataset_type] - }) + paths.append({"dataset_type": dataset_type, "path": path_templates[dataset_type]}) else: raise TypeNotFoundException(f"Dataset Type : {dataset_type} has no associated path.") return paths + # ------------------------------------------------------------------ + # File listings + # ------------------------------------------------------------------ + + def _get_dataset_file_paths(self, base_path, dataset_type): + """ + Return the explicit file listings for a dataset type under base_path. + + Index files (.fai, .gzi, .csi) and checksum files (md5sum.txt) are + intentionally excluded. + """ + if dataset_type == 'genebuild': + # geneset/ directory + filenames = [ + "cdna.fa.bgz", + "cdna.fa.gz", + "genes.embl.gz", + "genes.gff3.bgz", + "genes.gff3.gz", + "genes.gtf.bgz", + "genes.gtf.gz", + # The following are not alwalys present so we can't automatically generate them. No good solution for now. + # "genes-including_alt.embl.gz", + # "genes-including_alt.gff3.bgz", + # "genes-including_alt.gff3.gz", + # "genes-including_alt.gtf.bgz", + # "genes-including_alt.gtf.gz", # + "pep.fa.bgz", + "pep.fa.gz", + "xref.tsv.gz", + ] + return { + "annotations": {f: f"{base_path}/{f}" for f in filenames} + } + + elif dataset_type == 'assembly': + # genome/ directory + filenames = [ + "chromosomes.tsv.gz", + "hardmasked.fa.bgz", + "hardmasked.fa.gz", + "softmasked.fa.bgz", + "softmasked.fa.gz", + "unmasked.fa.bgz", + "unmasked.fa.gz", + ] + return { + "genome_sequences": {f: f"{base_path}/{f}" for f in filenames} + } + + elif dataset_type == 'homologies': + # homology/{partial_release}/ — single file + return { + "homology_data": { + "homology.tsv.gz": f"{base_path}/homology.tsv.gz" + } + } + + elif dataset_type == 'variation': + # variation/{partial_release}/ — single file + return { + "variation_data": { + "variation.vcf.gz": f"{base_path}/variation.vcf.gz" + } + } + + elif dataset_type == 'regulation': + return { + "regulatory_features": { + "regulation.gff": f"{base_path}/regulation.gff" + } + } + + return {} + + # ------------------------------------------------------------------ + # Helpers + # ------------------------------------------------------------------ + def _normalize_species_name(self, scientific_name): - """Normalize species name by replacing dots with underscores and merging multiple underscores.""" + """Normalise species name for use as a JSON key.""" scientific_name = re.sub(r'[^a-zA-Z0-9]+', ' ', scientific_name) scientific_name = scientific_name.replace(' ', '_') scientific_name = re.sub(r'^_+|_+$', '', scientific_name) return scientific_name def _extract_provider_from_path(self, genebuild_metadata): - """Extract the provider component from the genebuild metadata for use in paths.""" + """Return the annotation source component used as the provider key.""" if not genebuild_metadata or not genebuild_metadata.get('genebuild_source_name'): return 'unknown' - - provider = genebuild_metadata['genebuild_source_name'].lower() - return provider + return genebuild_metadata['genebuild_source_name'].lower() def _extract_genebuild_release_info(self, genebuild_metadata): - """Extract release information from genebuild metadata for use as release key/value.""" - release_info = { - "release": "unknown" - } - + """Extract the YYYY_MM geneset date for use as the release key in the JSON.""" + release_info = {"release": "unknown"} if genebuild_metadata and genebuild_metadata.get('last_geneset_update'): - last_geneset_update = genebuild_metadata['last_geneset_update'] - match = re.match(r'^(\d{4}-\d{2})', last_geneset_update) + match = re.match(r'^(\d{4}-\d{2})', genebuild_metadata['last_geneset_update']) if match: release_info["release"] = match.group(1).replace('-', '_') - return release_info def _extract_release_info_from_ensembl_release(self, genome): - """Extract release information from ensembl_release table.""" - release_info = { - "release": "unknown" - } - + """Extract release information from ensembl_release table (retained for reference).""" + release_info = {"release": "unknown"} for genome_release in genome.genome_releases: if genome_release.ensembl_release and genome_release.ensembl_release.status == ReleaseStatus.RELEASED: ensembl_release = genome_release.ensembl_release @@ -456,115 +616,25 @@ def _extract_release_info_from_ensembl_release(self, genome): elif hasattr(ensembl_release, 'name'): release_info["release"] = ensembl_release.name break - return release_info def _has_released_dataset_bulk(self, datasets, dataset_type): - """Check if there's a released dataset of the specified type using preloaded data.""" - + """Check if a released dataset of the given type exists in the preloaded data.""" type_mapping = { 'regulation': 'regulatory_features', 'genebuild': 'genebuild', 'assembly': 'assembly', 'homologies': 'homologies', - 'variation': 'variation' + 'variation': 'variation', } - mapped_type = type_mapping.get(dataset_type, dataset_type) - - return any( - d['dataset_type_name'] == mapped_type - for d in datasets - ) - - def _get_dataset_file_paths(self, base_path, dataset_type, genome, assembly_data): - """Generate specific file paths for a dataset type.""" - - file_paths = {} - - if dataset_type == 'genebuild': - file_paths = { - "annotations": { - "cdna.fa.gz": f"{base_path}/cdna.fa.gz", - "genes.embl.gz": f"{base_path}/genes.embl.gz", - "genes.gff3.gz": f"{base_path}/genes.gff3.gz", - "genes.gtf.gz": f"{base_path}/genes.gtf.gz", - "pep.fa.gz": f"{base_path}/pep.fa.gz", - "xref.tsv.gz": f"{base_path}/xref.tsv.gz" - } - } - - path_parts = base_path.split('/') - if len(path_parts) >= 4: - species = path_parts[0] - assembly = path_parts[1] - provider = path_parts[2] - release = path_parts[4] if len(path_parts) > 4 else path_parts[3] - vep_base = f"{species}/{assembly}/vep/{provider}/geneset/{release}" - - file_paths["vep"] = { - "genes.gff3.bgz": f"{vep_base}/genes.gff3.bgz", - "genes.gff3.bgz.csi": f"{vep_base}/genes.gff3.bgz.csi" - } - - elif dataset_type == 'assembly': - file_paths = { - "genome_sequences": { - "chromosomes.tsv.gz": f"{base_path}/chromosomes.tsv.gz", - "hardmasked.fa.gz": f"{base_path}/hardmasked.fa.gz", - "softmasked.fa.gz": f"{base_path}/softmasked.fa.gz", - "unmasked.fa.gz": f"{base_path}/unmasked.fa.gz" - } - } - - path_parts = base_path.split('/') - if len(path_parts) >= 2: - species = path_parts[0] - assembly = path_parts[1] - vep_genome_base = f"{species}/{assembly}/vep/genome" - - file_paths["vep"] = { - "softmasked.fa.bgz": f"{vep_genome_base}/softmasked.fa.bgz", - "softmasked.fa.bgz.fai": f"{vep_genome_base}/softmasked.fa.bgz.fai", - "softmasked.fa.bgz.gzi": f"{vep_genome_base}/softmasked.fa.bgz.gzi" - } - - elif dataset_type == 'homologies': - organism = genome.organism - assembly = genome.assembly - species_name = self._normalize_species_name(organism.scientific_name) - - release = base_path.split('/')[-1] if '/' in base_path else "unknown" - homology_filename = f"{species_name}-{assembly.accession}-{release}-homology.tsv.gz" - - file_paths = { - "homology_data": { - homology_filename: f"{base_path}/{homology_filename}" - } - } - - elif dataset_type == 'variation': - file_paths = { - "variation_data": { - "variation.vcf.gz": f"{base_path}/variation.vcf.gz" - } - } - - elif dataset_type == 'regulation': - file_paths = { - "regulatory_features": { - "regulation.gff": f"{base_path}/regulation.gff" - } - } - - return file_paths + return any(d['dataset_type_name'] == mapped_type for d in datasets) def main() -> None: """Main entry point for the script.""" - parser = argparse.ArgumentParser( - description="Generate index files for the ftp" + description="Generate index files for the FTP" ) parser.add_argument( "--metadata-uri", @@ -574,22 +644,21 @@ def main() -> None: parser.add_argument( "--output-path", default="species.json", - help="Optional output path for the stats files. Filenames will be: " - "species.json Defaults to current directory." + help="Output path for the JSON file. Defaults to species.json in the current directory." ) args = parser.parse_args() try: exporter = FTPMetadataExporter(metadata_uri=args.metadata_uri) - metadata = exporter.export_to_json(args.output_path) + exporter.export_to_json(args.output_path) print(f"Metadata exported to {args.output_path}") except ValueError as e: print(e) sys.exit(1) except Exception as e: - print(f"Error generating release statistics: {e}") + print(f"Error generating FTP metadata: {e}") sys.exit(1) if __name__ == "__main__": - main() + main() \ No newline at end of file diff --git a/src/tests/test_exports_json.py b/src/tests/test_exports_json.py index 422b4445..9183ddb2 100644 --- a/src/tests/test_exports_json.py +++ b/src/tests/test_exports_json.py @@ -16,7 +16,7 @@ import pytest -from ensembl.production.metadata.api.exports.ftp_index import FTPMetadataExporter +from ensembl.production.metadata.api.exports.ftp_index import FTPMetadataExporter, format_accession_path from ensembl.production.metadata.api.models import Genome, ReleaseStatus db_directory = Path(__file__).parent / 'databases' @@ -90,6 +90,23 @@ def test_load_all_genome_data(self, test_dbs): assert isinstance(first_genome_data['datasets'], list) assert isinstance(first_genome_data['attributes'], dict) + def test_load_all_genome_data_genebuild_metadata_fields(self, test_dbs): + """Test _load_all_genome_data populates genebuild_metadata with all required fields.""" + metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url + exporter = FTPMetadataExporter(metadata_uri) + with exporter.metadata_db.session_scope() as session: + genome_data = exporter._load_all_genome_data(session) + + for genome_uuid, data in genome_data.items(): + gm = data['genebuild_metadata'] + if gm is not None: + assert 'accession' in gm + assert 'genebuild_source_name' in gm + assert 'last_geneset_update' in gm + # These keys must always be present (value may be None if no partial release) + assert 'homology_release' in gm + assert 'variation_release' in gm + @pytest.mark.parametrize( ("input_name", "expected_name"), [ @@ -117,14 +134,8 @@ def test_extract_provider_from_path(self, test_dbs): """Test _extract_provider_from_path extracts provider correctly.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - genebuild_metadata = { - 'genebuild_source_name': 'Ensembl' - } - assert exporter._extract_provider_from_path(genebuild_metadata) == 'ensembl' - genebuild_metadata = { - 'genebuild_source_name': 'REFSEQ' - } - assert exporter._extract_provider_from_path(genebuild_metadata) == 'refseq' + assert exporter._extract_provider_from_path({'genebuild_source_name': 'Ensembl'}) == 'ensembl' + assert exporter._extract_provider_from_path({'genebuild_source_name': 'REFSEQ'}) == 'refseq' assert exporter._extract_provider_from_path(None) == 'unknown' assert exporter._extract_provider_from_path({}) == 'unknown' @@ -132,25 +143,11 @@ def test_extract_genebuild_release_info(self, test_dbs): """Test _extract_genebuild_release_info extracts release correctly.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - genebuild_metadata = { - 'last_geneset_update': '2024-01-01' - } - result = exporter._extract_genebuild_release_info(genebuild_metadata) - assert result['release'] == '2024_01' - genebuild_metadata = { - 'last_geneset_update': '2023-12-15' - } - result = exporter._extract_genebuild_release_info(genebuild_metadata) - assert result['release'] == '2023_12' - result = exporter._extract_genebuild_release_info(None) - assert result['release'] == 'unknown' - result = exporter._extract_genebuild_release_info({}) - assert result['release'] == 'unknown' - genebuild_metadata = { - 'last_geneset_update': 'invalid-date' - } - result = exporter._extract_genebuild_release_info(genebuild_metadata) - assert result['release'] == 'unknown' + assert exporter._extract_genebuild_release_info({'last_geneset_update': '2024-01-01'})['release'] == '2024_01' + assert exporter._extract_genebuild_release_info({'last_geneset_update': '2023-12-15'})['release'] == '2023_12' + assert exporter._extract_genebuild_release_info(None)['release'] == 'unknown' + assert exporter._extract_genebuild_release_info({})['release'] == 'unknown' + assert exporter._extract_genebuild_release_info({'last_geneset_update': 'invalid-date'})['release'] == 'unknown' def test_extract_release_info_from_ensembl_release(self, test_dbs): """Test _extract_release_info_from_ensembl_release extracts release correctly.""" @@ -158,10 +155,8 @@ def test_extract_release_info_from_ensembl_release(self, test_dbs): exporter = FTPMetadataExporter(metadata_uri) with exporter.metadata_db.session_scope() as session: genome = session.query(Genome).first() - if genome: result = exporter._extract_release_info_from_ensembl_release(genome) - assert isinstance(result, dict) assert 'release' in result has_released = any( @@ -176,104 +171,143 @@ def test_has_released_dataset_bulk(self, test_dbs): metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - datasets = [ - {'dataset_type_name': 'genebuild'}, - {'dataset_type_name': 'assembly'} - ] + datasets = [{'dataset_type_name': 'genebuild'}, {'dataset_type_name': 'assembly'}] assert exporter._has_released_dataset_bulk(datasets, 'genebuild') is True assert exporter._has_released_dataset_bulk(datasets, 'assembly') is True assert exporter._has_released_dataset_bulk(datasets, 'variation') is False - datasets = [ - {'dataset_type_name': 'regulatory_features'} - ] - assert exporter._has_released_dataset_bulk(datasets, 'regulation') is True + assert exporter._has_released_dataset_bulk([{'dataset_type_name': 'regulatory_features'}], 'regulation') is True assert exporter._has_released_dataset_bulk([], 'genebuild') is False + # ------------------------------------------------------------------ + # format_accession_path + # ------------------------------------------------------------------ + + @pytest.mark.parametrize( + ("accession", "expected_path"), + [ + ("GCA_000001405.29", "GCA/000/001/405/29"), + ("GCF_000001405.40", "GCF/000/001/405/40"), + ("GCA_003339765.3", "GCA/003/339/765/3"), + ("GCA_000005845.2", "GCA/000/005/845/2"), + ], + ) + def test_format_accession_path(self, test_dbs, accession, expected_path): + """Test format_accession_path converts accessions to correct directory paths.""" + assert format_accession_path(accession) == expected_path + + def test_format_accession_path_invalid(self, test_dbs): + """Test format_accession_path raises ValueError for invalid accessions.""" + with pytest.raises(ValueError, match="Invalid accession format"): + format_accession_path("GRCh38") + with pytest.raises(ValueError, match="Invalid accession format"): + format_accession_path("GCA_00000140529") # missing dot-version + with pytest.raises(ValueError, match="Invalid accession format"): + format_accession_path("") + + # ------------------------------------------------------------------ + # _get_dataset_file_paths + # ------------------------------------------------------------------ + def test_get_dataset_file_paths_genebuild(self, test_dbs): """Test _get_dataset_file_paths generates correct file paths for genebuild.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - base_path = "homo_sapiens/GRCh38/ensembl/geneset/2024_01" - with exporter.metadata_db.session_scope() as session: - genome = session.query(Genome).first() - assembly_data = {'accession': 'GRCh38'} if genome else {} - file_paths = exporter._get_dataset_file_paths( - base_path, 'genebuild', genome, assembly_data - ) + base_path = "GCA/000/001/405/29/ensembl/2024_01/geneset" + file_paths = exporter._get_dataset_file_paths(base_path, 'genebuild') assert 'annotations' in file_paths - assert 'cdna.fa.gz' in file_paths['annotations'] - assert 'genes.gff3.gz' in file_paths['annotations'] - assert 'genes.gtf.gz' in file_paths['annotations'] - assert 'pep.fa.gz' in file_paths['annotations'] - assert 'vep' in file_paths - assert 'genes.gff3.bgz' in file_paths['vep'] + annotations = file_paths['annotations'] + # Core files always present + for fname in ('cdna.fa.gz', 'cdna.fa.bgz', 'genes.embl.gz', + 'genes.gff3.gz', 'genes.gff3.bgz', + 'genes.gtf.gz', 'genes.gtf.bgz', + 'pep.fa.gz', 'pep.fa.bgz', 'xref.tsv.gz'): + assert fname in annotations, f"Expected {fname} in genebuild annotations" + # Paths should be rooted under base_path + assert annotations['cdna.fa.gz'] == f"{base_path}/cdna.fa.gz" + # Index files must NOT be present + for fname in annotations: + assert not fname.endswith('.fai'), f"Index file {fname} should not be listed" + assert not fname.endswith('.gzi'), f"Index file {fname} should not be listed" + assert not fname.endswith('.csi'), f"Index file {fname} should not be listed" + # No VEP section in new structure + assert 'vep' not in file_paths + # genes-including_alt files are intentionally omitted (not always present) + for fname in annotations: + assert 'including_alt' not in fname def test_get_dataset_file_paths_assembly(self, test_dbs): """Test _get_dataset_file_paths generates correct file paths for assembly.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - base_path = "homo_sapiens/GRCh38/genome" - with exporter.metadata_db.session_scope() as session: - genome = session.query(Genome).first() - assembly_data = {'accession': 'GRCh38'} if genome else {} - file_paths = exporter._get_dataset_file_paths( - base_path, 'assembly', genome, assembly_data - ) + base_path = "GCA/000/001/405/29/ensembl/2024_01/genome" + file_paths = exporter._get_dataset_file_paths(base_path, 'assembly') + assert 'genome_sequences' in file_paths - assert 'chromosomes.tsv.gz' in file_paths['genome_sequences'] - assert 'hardmasked.fa.gz' in file_paths['genome_sequences'] - assert 'softmasked.fa.gz' in file_paths['genome_sequences'] - assert 'unmasked.fa.gz' in file_paths['genome_sequences'] - assert 'vep' in file_paths - assert 'softmasked.fa.bgz' in file_paths['vep'] + genome_seqs = file_paths['genome_sequences'] + for fname in ('chromosomes.tsv.gz', + 'hardmasked.fa.bgz', 'hardmasked.fa.gz', + 'softmasked.fa.bgz', 'softmasked.fa.gz', + 'unmasked.fa.bgz', 'unmasked.fa.gz'): + assert fname in genome_seqs, f"Expected {fname} in genome_sequences" + assert genome_seqs['softmasked.fa.gz'] == f"{base_path}/softmasked.fa.gz" + # Index files must NOT be present + for fname in genome_seqs: + assert not fname.endswith('.fai') + assert not fname.endswith('.gzi') + # No VEP section in new structure + assert 'vep' not in file_paths + # md5sum must NOT be present + assert 'md5sum.txt' not in genome_seqs + + def test_get_dataset_file_paths_homologies(self, test_dbs): + """Test _get_dataset_file_paths generates correct file paths for homologies.""" + metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url + exporter = FTPMetadataExporter(metadata_uri) + base_path = "GCA/000/001/405/29/ensembl/2024_01/homology/2024_10_18" + file_paths = exporter._get_dataset_file_paths(base_path, 'homologies') + + assert 'homology_data' in file_paths + homology_files = file_paths['homology_data'] + assert 'homology.tsv.gz' in homology_files + assert homology_files['homology.tsv.gz'] == f"{base_path}/homology.tsv.gz" + # Only one file in the terminal directory + assert len(homology_files) == 1 def test_get_dataset_file_paths_variation(self, test_dbs): """Test _get_dataset_file_paths generates correct file paths for variation.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - base_path = "homo_sapiens/GRCh38/ensembl/variation/2024_01" - with exporter.metadata_db.session_scope() as session: - genome = session.query(Genome).first() - assembly_data = {'accession': 'GRCh38'} if genome else {} - file_paths = exporter._get_dataset_file_paths( - base_path, 'variation', genome, assembly_data - ) + base_path = "GCA/000/001/405/29/ensembl/2024_01/variation/2024_10_18" + file_paths = exporter._get_dataset_file_paths(base_path, 'variation') + assert 'variation_data' in file_paths - assert 'variation.vcf.gz' in file_paths['variation_data'] + variation_files = file_paths['variation_data'] + assert 'variation.vcf.gz' in variation_files + assert variation_files['variation.vcf.gz'] == f"{base_path}/variation.vcf.gz" + assert len(variation_files) == 1 def test_get_dataset_file_paths_regulation(self, test_dbs): """Test _get_dataset_file_paths generates correct file paths for regulation.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - base_path = "homo_sapiens/GRCh38/ensembl/regulation" - with exporter.metadata_db.session_scope() as session: - genome = session.query(Genome).first() - assembly_data = {'accession': 'GRCh38'} if genome else {} - file_paths = exporter._get_dataset_file_paths( - base_path, 'regulation', genome, assembly_data - ) + base_path = "GCA/000/001/405/29/ensembl/2024_01/regulation" + file_paths = exporter._get_dataset_file_paths(base_path, 'regulation') assert 'regulatory_features' in file_paths assert 'regulation.gff' in file_paths['regulatory_features'] + assert file_paths['regulatory_features']['regulation.gff'] == f"{base_path}/regulation.gff" - def test_get_dataset_file_paths_homologies(self, test_dbs): - """Test _get_dataset_file_paths generates correct file paths for homologies.""" + def test_get_dataset_file_paths_unknown_type(self, test_dbs): + """Test _get_dataset_file_paths returns empty dict for unknown dataset type.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url exporter = FTPMetadataExporter(metadata_uri) - base_path = "homo_sapiens/GRCh38/ensembl/homology/2024_01" - with exporter.metadata_db.session_scope() as session: - genome = session.query(Genome).first() - if genome: - assembly_data = {'accession': genome.assembly.accession} - file_paths = exporter._get_dataset_file_paths( - base_path, 'homologies', genome, assembly_data - ) - assert 'homology_data' in file_paths - homology_files = file_paths['homology_data'] - assert len(homology_files) > 0 - first_file = next(iter(homology_files.keys())) - assert 'homology.tsv.gz' in first_file + file_paths = exporter._get_dataset_file_paths("some/path", 'nonexistent_type') + assert file_paths == {} + + # ------------------------------------------------------------------ + # Integration / JSON structure + # ------------------------------------------------------------------ def test_export_json_with_actual_data(self, test_dbs): """Test export generates valid JSON structure with actual database data.""" @@ -287,16 +321,31 @@ def test_export_json_with_actual_data(self, test_dbs): for species_name, species_data in metadata['species'].items(): assert isinstance(species_name, str) assert 'assemblies' in species_data - for assembly_name, assembly_data in species_data['assemblies'].items(): - assert isinstance(assembly_name, str) - if 'providers' in assembly_data: - for provider_name, provider_data in assembly_data['providers'].items(): - assert isinstance(provider_name, str) - if 'releases' in provider_data: - for release_name, release_data in provider_data['releases'].items(): - assert isinstance(release_name, str) - if 'datasets' in release_data: - assert isinstance(release_data['datasets'], dict) + for assembly_accession, assembly_data in species_data['assemblies'].items(): + assert isinstance(assembly_accession, str) + assert 'genebuild_providers' in assembly_data + for provider_name, provider_data in assembly_data['genebuild_providers'].items(): + assert isinstance(provider_name, str) + for release_key, release_data in provider_data.items(): + assert isinstance(release_key, str) + assert 'release' in release_data + assert 'paths' in release_data + + def test_export_assembly_paths_use_accession_format(self, test_dbs): + """Test that generated paths use the new GCA/NNN/NNN/NNN/VV accession format.""" + metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url + exporter = FTPMetadataExporter(metadata_uri) + metadata = exporter.export_to_json() + + for species_data in metadata['species'].values(): + for assembly_data in species_data['assemblies'].values(): + if assembly_data.get('assembly'): + for category in assembly_data['assembly']['files'].values(): + for path in category.values(): + # Path must start with GCA/ or GCF/ followed by 3-digit chunks + assert path.startswith('GCA/') or path.startswith('GCF/'), ( + f"Path does not use accession format: {path}" + ) def test_export_handles_empty_database(self, test_dbs): """Test export handles database with no released genomes gracefully.""" @@ -319,4 +368,4 @@ def test_json_file_is_valid_json(self, test_dbs, tmp_path): data = json.load(f) assert 'last_updated' in data assert 'species' in data - datetime.fromisoformat(data['last_updated']) + datetime.fromisoformat(data['last_updated']) \ No newline at end of file From 0b48c5dc298ab49a8d2965e10e1c2f17c4b4efcd Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Tue, 12 May 2026 10:46:02 +0100 Subject: [PATCH 06/10] :bug: fix unexpected error: Multiple rows were found when one or none was required Cause: GenomeAdaptor.get_public_path() used scalar_one_or_none() for variation/homology release lookup, but the DB can validly return multiple matching rows. Since search asks only for dataset_type="genebuild", those variation/homology lookups were unnecessary anyway. Changes made: - In genome.py (line 1115), variation/homology release lookup now only runs when dataset_type is all, variation, or homologies - Replaced scalar_one_or_none() with deterministic .order_by(EnsemblRelease.release_id.desc()).scalars().first() --- .../metadata/api/adaptors/genome.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ensembl/production/metadata/api/adaptors/genome.py b/src/ensembl/production/metadata/api/adaptors/genome.py index 18d13231..9e31efbb 100644 --- a/src/ensembl/production/metadata/api/adaptors/genome.py +++ b/src/ensembl/production/metadata/api/adaptors/genome.py @@ -1112,7 +1112,7 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): # === RELEASE HANDLING === if release is None: - if 'variation' in unique_dataset_types: + if 'variation' in unique_dataset_types and dataset_type in ('all', 'variation'): variation_release = session.execute( select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( Genome).where( @@ -1121,10 +1121,10 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): Dataset.status == DatasetStatus.RELEASED, GenomeDataset.is_current == True, EnsemblRelease.release_type == 'partial' - ) - ).scalar_one_or_none() + ).order_by(EnsemblRelease.release_id.desc()) + ).scalars().first() - if 'homologies' in unique_dataset_types: + if 'homologies' in unique_dataset_types and dataset_type in ('all', 'homologies'): homology_release = session.execute( select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( Genome).where( @@ -1133,10 +1133,10 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): Dataset.status == DatasetStatus.RELEASED, GenomeDataset.is_current == True, EnsemblRelease.release_type == 'partial' - ) - ).scalar_one_or_none() + ).order_by(EnsemblRelease.release_id.desc()) + ).scalars().first() else: - if 'variation' in unique_dataset_types: + if 'variation' in unique_dataset_types and dataset_type in ('all', 'variation'): variation_release = session.execute( select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( Genome).where( @@ -1149,12 +1149,12 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): EnsemblRelease.label == release).scalar_subquery() ) ).order_by(EnsemblRelease.release_id.desc()) - ).scalar_one_or_none() + ).scalars().first() if variation_release is None: unique_dataset_types = [t for t in unique_dataset_types if t != 'variation'] - if 'homologies' in unique_dataset_types: + if 'homologies' in unique_dataset_types and dataset_type in ('all', 'homologies'): homology_release = session.execute( select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( Genome).where( @@ -1167,7 +1167,7 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): EnsemblRelease.label == release).scalar_subquery() ) ).order_by(EnsemblRelease.release_id.desc()) - ).scalar_one_or_none() + ).scalars().first() if homology_release is None: unique_dataset_types = [t for t in unique_dataset_types if t != 'homologies'] From 65162795705f5ac048fc18e5b6b1859412228639 Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Tue, 12 May 2026 10:48:42 +0100 Subject: [PATCH 07/10] :pencil: remove slicing gid for testing purposes --- src/ensembl/production/metadata/api/search/search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ensembl/production/metadata/api/search/search.py b/src/ensembl/production/metadata/api/search/search.py index 836fd9b5..9454c0c9 100644 --- a/src/ensembl/production/metadata/api/search/search.py +++ b/src/ensembl/production/metadata/api/search/search.py @@ -664,7 +664,7 @@ def _get_genome_ids_to_process(self, session: Session) -> List[int]: .distinct() .all() ) - return [gid[0] for gid in genome_ids][:10] + return [gid[0] for gid in genome_ids] def _get_genomes_batch(self, session: Session, genome_ids: List[int]) -> List[Genome]: """ From 74f41bf157d30a9f4afedea9255a72d8a3a2b975 Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Mon, 18 May 2026 13:58:36 +0100 Subject: [PATCH 08/10] :white_check_mark: fix broken test --- src/tests/test_search.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/test_search.py b/src/tests/test_search.py index 83504fd1..1ad05965 100644 --- a/src/tests/test_search.py +++ b/src/tests/test_search.py @@ -260,6 +260,7 @@ def test_document_serializes_genome_group_ids_as_repeated_fields(self, test_dbs) scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, From 73fe550ff6023dcc31a3cc6f735195faec7087c8 Mon Sep 17 00:00:00 2001 From: danielp Date: Thu, 2 Jul 2026 11:09:21 +0100 Subject: [PATCH 09/10] Merged main back in and then updated the code to match what it should do. Needs verification for search and the ftp. --- .../metadata/api/adaptors/genome.py | 84 ++++++++++++------- .../metadata/api/exports/ftp_index.py | 26 ++---- src/tests/test_exports_json.py | 18 +--- src/tests/test_search.py | 1 + 4 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/ensembl/production/metadata/api/adaptors/genome.py b/src/ensembl/production/metadata/api/adaptors/genome.py index 02399dd8..97d05cf0 100644 --- a/src/ensembl/production/metadata/api/adaptors/genome.py +++ b/src/ensembl/production/metadata/api/adaptors/genome.py @@ -1152,7 +1152,7 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): Args: genome_uuid (str): Unique identifier for the genome - dataset_type (str): Type of dataset ('genebuild', 'assembly', 'homologies', 'variation', or 'all') + dataset_type (str): Type of dataset ('genebuild', 'assembly', 'homologies', 'short_variants', or 'all') release (str, optional): Specific Ensembl release label. If None, uses current release. Returns: @@ -1167,6 +1167,8 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): accession = None genebuild_source_name = None last_geneset_update = None + if dataset_type == "variation": + dataset_type = "short_variants" with self.metadata_db.session_scope() as session: # === VALIDATION SECTION === @@ -1230,17 +1232,26 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): # === RELEASE HANDLING === if release is None: - if 'variation' in unique_dataset_types and dataset_type in ('all', 'variation'): - variation_release = session.execute( - select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( - Genome).where( - Genome.genome_uuid == genome_uuid, - DatasetType.name == 'variation', - Dataset.status == DatasetStatus.RELEASED, - GenomeDataset.is_current == True, - EnsemblRelease.release_type == 'partial' - ).order_by(EnsemblRelease.release_id.desc()) - ).scalars().first() + if "short_variants" in unique_dataset_types and dataset_type in ("all", "short_variants"): + variation_release = ( + session.execute( + select(EnsemblRelease.label) + .join(GenomeDataset) + .join(Dataset) + .join(DatasetType) + .join(Genome) + .where( + Genome.genome_uuid == genome_uuid, + DatasetType.name == "short_variants", + Dataset.status == DatasetStatus.RELEASED, + GenomeDataset.is_current == True, + EnsemblRelease.release_type == "partial", + ) + .order_by(EnsemblRelease.release_id.desc()) + ) + .scalars() + .first() + ) if 'homologies' in unique_dataset_types and dataset_type in ('all', 'homologies'): homology_release = session.execute( @@ -1254,23 +1265,34 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): ).order_by(EnsemblRelease.release_id.desc()) ).scalars().first() else: - if 'variation' in unique_dataset_types and dataset_type in ('all', 'variation'): - variation_release = session.execute( - select(EnsemblRelease.label).join(GenomeDataset).join(Dataset).join(DatasetType).join( - Genome).where( - Genome.genome_uuid == genome_uuid, - DatasetType.name == 'variation', - Dataset.status == DatasetStatus.RELEASED, - EnsemblRelease.release_type == 'partial', - EnsemblRelease.release_id <= ( - select(EnsemblRelease.release_id).where( - EnsemblRelease.label == release).scalar_subquery() + if "short_variants" in unique_dataset_types and dataset_type in ("all", "short_variants"): + variation_release = ( + session.execute( + select(EnsemblRelease.label) + .join(GenomeDataset) + .join(Dataset) + .join(DatasetType) + .join(Genome) + .where( + Genome.genome_uuid == genome_uuid, + DatasetType.name == "short_variants", + Dataset.status == DatasetStatus.RELEASED, + EnsemblRelease.release_type == "partial", + EnsemblRelease.release_id + <= ( + select(EnsemblRelease.release_id) + .where(EnsemblRelease.label == release) + .scalar_subquery() + ), ) - ).order_by(EnsemblRelease.release_id.desc()) - ).scalars().first() + .order_by(EnsemblRelease.release_id.desc()) + ) + .scalars() + .first() + ) if variation_release is None: - unique_dataset_types = [t for t in unique_dataset_types if t != 'variation'] + unique_dataset_types = [t for t in unique_dataset_types if t != "short_variants"] if 'homologies' in unique_dataset_types and dataset_type in ('all', 'homologies'): homology_release = session.execute( @@ -1311,14 +1333,14 @@ def get_public_path(self, genome_uuid, dataset_type='all', release=None): if 'homologies' in unique_dataset_types and homology_release: homology_release = homology_release.replace('-', '_') - if 'variation' in unique_dataset_types and variation_release: + if "short_variants" in unique_dataset_types and variation_release: variation_release = variation_release.replace('-', '_') path_templates = { - 'genebuild': f"{common_path}/geneset", - 'assembly': f"{common_path}/genome", - 'homologies': f"{common_path}/homology/{homology_release}", - 'variation': f"{common_path}/variation/{variation_release}", + "genebuild": f"{common_path}/geneset", + "assembly": f"{common_path}/genome", + "homologies": f"{common_path}/homology/{homology_release}", + "short_variants": f"{common_path}/variation/{variation_release}", } # === REQUEST VALIDATION === diff --git a/src/ensembl/production/metadata/api/exports/ftp_index.py b/src/ensembl/production/metadata/api/exports/ftp_index.py index 29cfb2a4..3644f6c0 100644 --- a/src/ensembl/production/metadata/api/exports/ftp_index.py +++ b/src/ensembl/production/metadata/api/exports/ftp_index.py @@ -459,18 +459,13 @@ def _get_public_paths_bulk(self, genebuild_metadata, datasets, dataset_type='all variation_release = variation_release.replace('-', '_') # Collect the dataset types actually present for this genome - unique_dataset_types = list(set([ - 'regulation' if d['dataset_type_name'] == 'regulatory_features' - else d['dataset_type_name'] - for d in datasets - ])) + unique_dataset_types = list(set([d["dataset_type_name"] for d in datasets])) # Drop homologies / variation if there is no partial release date available if not homology_release and 'homologies' in unique_dataset_types: unique_dataset_types = [t for t in unique_dataset_types if t != 'homologies'] - if not variation_release and 'short_variants' in unique_dataset_types: - unique_dataset_types = [t for t in unique_dataset_types if t != 'variation'] - + if not variation_release and "short_variants" in unique_dataset_types: + unique_dataset_types = [t for t in unique_dataset_types if t != "short_variants"] # Build paths accession_path = format_accession_path(accession) @@ -503,13 +498,14 @@ def _get_public_paths_bulk(self, genebuild_metadata, datasets, dataset_type='all # File listings # ------------------------------------------------------------------ - def _get_dataset_file_paths(self, base_path, dataset_type): + def _get_dataset_file_paths(self, base_path, dataset_type, *_, **__): """ Return the explicit file listings for a dataset type under base_path. Index files (.fai, .gzi, .csi) and checksum files (md5sum.txt) are intentionally excluded. """ + if dataset_type == 'genebuild': # geneset/ directory filenames = [ @@ -564,14 +560,6 @@ def _get_dataset_file_paths(self, base_path, dataset_type): "variation.vcf.gz": f"{base_path}/variation.vcf.gz" } } - - elif dataset_type == 'regulation': - return { - "regulatory_features": { - "regulation.gff": f"{base_path}/regulation.gff" - } - } - return {} # ------------------------------------------------------------------ @@ -621,7 +609,7 @@ def _has_released_dataset_bulk(self, datasets, dataset_type): "genebuild": "genebuild", "assembly": "assembly", "homologies": "homologies", - "variation": "short_variants", + "short_variants": "short_variants", } mapped_type = type_mapping.get(dataset_type, dataset_type) return any(d['dataset_type_name'] == mapped_type for d in datasets) @@ -657,4 +645,4 @@ def main() -> None: if __name__ == "__main__": - main() \ No newline at end of file + main() diff --git a/src/tests/test_exports_json.py b/src/tests/test_exports_json.py index 02346e18..dacc6777 100644 --- a/src/tests/test_exports_json.py +++ b/src/tests/test_exports_json.py @@ -175,7 +175,6 @@ def test_has_released_dataset_bulk(self, test_dbs): assert exporter._has_released_dataset_bulk(datasets, 'genebuild') is True assert exporter._has_released_dataset_bulk(datasets, 'assembly') is True assert exporter._has_released_dataset_bulk(datasets, 'variation') is False - assert exporter._has_released_dataset_bulk([{'dataset_type_name': 'regulatory_features'}], 'regulation') is True assert exporter._has_released_dataset_bulk([], 'genebuild') is False # ------------------------------------------------------------------ @@ -282,26 +281,13 @@ def test_get_dataset_file_paths_variation(self, test_dbs): with exporter.metadata_db.session_scope() as session: genome = session.query(Genome).first() assembly_data = {'accession': 'GRCh38'} if genome else {} - file_paths = exporter._get_dataset_file_paths( - base_path, 'variation', genome, assembly_data - ) + file_paths = exporter._get_dataset_file_paths(base_path, "short_variants", genome, assembly_data) assert 'variation_data' in file_paths variation_files = file_paths['variation_data'] assert 'variation.vcf.gz' in variation_files assert variation_files['variation.vcf.gz'] == f"{base_path}/variation.vcf.gz" assert len(variation_files) == 1 - def test_get_dataset_file_paths_regulation(self, test_dbs): - """Test _get_dataset_file_paths generates correct file paths for regulation.""" - metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url - exporter = FTPMetadataExporter(metadata_uri) - base_path = "GCA/000/001/405/29/ensembl/2024_01/regulation" - file_paths = exporter._get_dataset_file_paths(base_path, 'regulation') - - assert 'regulatory_features' in file_paths - assert 'regulation.gff' in file_paths['regulatory_features'] - assert file_paths['regulatory_features']['regulation.gff'] == f"{base_path}/regulation.gff" - def test_get_dataset_file_paths_unknown_type(self, test_dbs): """Test _get_dataset_file_paths returns empty dict for unknown dataset type.""" metadata_uri = test_dbs['ensembl_genome_metadata'].dbc.url @@ -372,4 +358,4 @@ def test_json_file_is_valid_json(self, test_dbs, tmp_path): data = json.load(f) assert 'last_updated' in data assert 'species' in data - datetime.fromisoformat(data['last_updated']) \ No newline at end of file + datetime.fromisoformat(data['last_updated']) diff --git a/src/tests/test_search.py b/src/tests/test_search.py index 97e7bd01..63b90fb6 100644 --- a/src/tests/test_search.py +++ b/src/tests/test_search.py @@ -295,6 +295,7 @@ def test_document_serializes_lineage_as_repeated_fields(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", + ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, From 8cab8f7b65ee0ad84f3c2c4b5adc898a6eaf6c9a Mon Sep 17 00:00:00 2001 From: Bilal El Houdaigui Date: Thu, 9 Jul 2026 11:38:18 +0100 Subject: [PATCH 10/10] :bug: update FTP URLs to use HTTP instead of HTTPS in search and test files --- .../production/metadata/api/search/search.py | 2 +- src/tests/test_search.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ensembl/production/metadata/api/search/search.py b/src/ensembl/production/metadata/api/search/search.py index 5699bc1c..741c82c4 100644 --- a/src/ensembl/production/metadata/api/search/search.py +++ b/src/ensembl/production/metadata/api/search/search.py @@ -652,7 +652,7 @@ def validate_unique_url_names(entries: List[SearchEntry]) -> None: class GenomeSearchIndexer: """Service for generating genome search documents""" - FTP_BASE_URL = "https://ftp.ebi.ac.uk/pub/ensemblorganisms" + FTP_BASE_URL = "http://ftp.ebi.ac.uk/pub/ensemblorganisms" def __init__(self, metadata_uri: str, taxonomy_uri: str, batch_size: int = 500): self.metadata_db = DBConnection(metadata_uri) diff --git a/src/tests/test_search.py b/src/tests/test_search.py index 63b90fb6..70d71e72 100644 --- a/src/tests/test_search.py +++ b/src/tests/test_search.py @@ -181,7 +181,7 @@ def test_valid_document_creation(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -221,7 +221,7 @@ def test_document_with_optional_fields(self, test_dbs): strain="GRCh38", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", url_name="homo_sapiens", tol_id="mHomSap1", is_reference=True, @@ -262,7 +262,7 @@ def test_document_serializes_genome_group_ids_as_repeated_fields(self, test_dbs) scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -295,7 +295,7 @@ def test_document_serializes_lineage_as_repeated_fields(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -369,7 +369,7 @@ def test_document_model_dump(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -402,7 +402,7 @@ def test_rank_injection(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606, @@ -427,7 +427,7 @@ def test_rank_injection(self, test_dbs): scientific_name="Homo sapiens", assembly_name="GRCh38", accession="GCA_000001405.15", - ftp="https://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", + ftp="http://ftp.ebi.ac.uk/pub/ensemblorganisms/GCA/000/001/405/15/ensembl/2022_07/", is_reference=True, species_taxonomy_id=9606, taxonomy_id=9606,