Merge branch 'master' into fix-fst-y-range-721

Author: jonbrenas

AI-POLICY.md

@@ -0,0 +1,27 @@
+# AI use policy and guidelines
+
+The goal of the MalariaGEN data API is to make access, use, and interpretation of the genomic data collected by our partners as easy and intuitive as possible. Maintainers have limited time and attention to focus on reviews, which means that each review request has to be for code that you can be proud of.
+
+Any tool that can help produce better code and understand better the existing codebase, including AI tools, can be used. The only key questions are: “Is this an improvement?” and “Why is the code better now?”.
+
+NEVER submit an AI-generated PR if you are not able to understand and explain the changes and why they matter. Maintainers WILL close PRs without reviewing them if they feel like they are a waste of time.
+
+## Using AI as a coding assistant
+
+1.	Understanding and familiarising yourself with the codebase is key. No matter how good the AI code assistant, it will return useless code if you do not provide a smart and accurate enough prompt.
+2.	Always check that your changes make sense. LLMs are terrible at saying no to a prompt and will lie and make false claims if they can’t do otherwise. It is particularly true if they lack key information.
+3.	Each commit should be its own piece of coherent change. LLMs like to do everything at once but digestible change is easier to understand and process.
+4.	Commenting your code is important, but LLMs really like to listen to themselves talking and will be very verbose. A small comment explaining why you made a choice is better than a paragraph explaining how a loop iterates through a list.
+
+## Using AI for communication
+
+As noted above, maintainers have a limited amount of time to spend on malariaGEN data API maintenance and do not want to waste it going through long, sloppy PR descriptions of simple issue. We strongly prefer clear and concise communication, even if it means we have to ask questions when more details are needed.
+
+You are responsible for your own PRs and comments. Even if you use an LLM to write a PR description or comment, you are expected to read through everything and make sure that it accurately and concisely reflects your opinions, ideas and contributions. If reading your own PRs and comments is too much work for you, it is going to be the same for everyone else.
+Here are some concrete guidelines for using AI as part of your communication toolbox.
+
+1.	In general, the question that needs answering is why not what. Maintainers can see the files and lines of codes that were modified, what they will want to know is the reasoning behind the choices. Sadly, LLMs are not great at explaining their reasoning so you probably will have to chip in.
+2.	In the same way, if you are responding to a comment or a review, you will need to justify your choice and explain how you made the decision.
+3.	Make sure that the description of your work is accurate. Errors can happen but it is fairly obvious when an LLM claims more than it delivers.
+4.	We are aware that English is not everyone’s first language. The grammar of your communications isn’t as important as the quality of your contribution. Feel free to use AI to improve your writing style but make sure that you still understand the message, that its content is conserved and that it doesn’t turn into an epic poem.
+5.	Maintainers are more interested in your ideas and thoughts than in the standard answer provided by an LLM. We work with genomic data, and contributors are not expected to be experts in computer science, software engineering, genomics, entomology, …  You are allowed not to know or not to be sure and it is miles better to say so than it is to regurgitate an answer that you do not understand.

README.md

@@ -46,88 +46,11 @@ for release notes.
 
 To get setup for development, see [this video if you prefer VS Code](https://youtu.be/zddl3n1DCFM), or [this older video if you prefer PyCharm](https://youtu.be/QniQi-Hoo9A), and the instructions below.
 
-Fork and clone this repo:
+Detailed instructions can be found in the [Contributors guide](https://github.com/malariagen/malariagen-data-python/blob/master/CONTRIBUTING.md).
 
-```bash
-git clone git@github.com:[username]/malariagen-data-python.git
-```
-
-Install Python, e.g.:
-
-```bash
-sudo add-apt-repository ppa:deadsnakes/ppa
-sudo apt install python3.10 python3.10-venv
-```
-
-Install pipx, e.g.:
-
-```bash
-python3.10 -m pip install --user pipx
-python3.10 -m pipx ensurepath
-```
-
-Install [poetry](https://python-poetry.org/docs/#installation), e.g.:
-
-```bash
-pipx install poetry
-```
-
-Create development environment:
-
-```bash
-cd malariagen-data-python
-poetry use 3.10
-poetry install
-```
-
-Activate development environment:
-
-```bash
-poetry shell
-```
-
-Install pre-commit and pre-commit hooks:
-
-```bash
-pipx install pre-commit
-pre-commit install
-```
-
-Run pre-commit checks (isort, black, blackdoc, flake8, ...) manually:
-
-```bash
-pre-commit run --all-files
-```
-
-Run fast unit tests using simulated data:
-
-```bash
-poetry run pytest -v tests/anoph
-```
-
-To run legacy tests which read data from GCS, you'll need to [request access to MalariaGEN data on GCS](https://malariagen.github.io/vector-data/vobs/vobs-data-access.html).
-
-Once access has been granted, [install the Google Cloud CLI](https://cloud.google.com/sdk/docs/install). E.g., if on Linux:
-
-```bash
-./install_gcloud.sh
-```
-
-You'll then need to obtain application-default credentials, e.g.:
-
-```bash
-./google-cloud-sdk/bin/gcloud auth application-default login
-```
-
-Once this is done, you can run legacy tests:
-
-```bash
-poetry run pytest --ignore=tests/anoph -v tests
-```
+## AI use policy and guidelines
 
-Tests will run slowly the first time, as data required for testing
-will be read from GCS. Subsequent runs will be faster as data will be
-cached locally in the "gcs_cache" folder.
+See [AI use policy and guidelines](https://github.com/malariagen/malariagen-data-python/blob/master/AI-POLICY.md) for more details.
 
 ## Release process
 

malariagen_data/amin1.py

@@ -105,7 +105,7 @@ def __init__(
             site_filters_analysis=site_filters_analysis,
             discordant_read_calls_analysis=discordant_read_calls_analysis,
             default_site_mask="minimus",
-            default_phasing_analysis="minimus_noneyet",
+            default_phasing_analysis=None,
             default_coverage_calls_analysis="minimus_noneyet",
             bokeh_output_notebook=bokeh_output_notebook,
             results_cache=results_cache,

malariagen_data/anoph/frq_base.py

@@ -276,7 +276,7 @@ def plot_frequencies_heatmap(
         heatmap_df.set_index(index_col, inplace=True)
 
         # Clean column names.
-        heatmap_df.columns = heatmap_df.columns.str.lstrip("frq_")
+        heatmap_df.columns = heatmap_df.columns.str.removeprefix("frq_")
 
         # Deal with width and height.
         if width is None:
@@ -385,8 +385,8 @@ def plot_frequencies_time_series(
 
         # Build a long-form dataframe from the dataset.
         dfs = []
-        for cohort_index, cohort in enumerate(df_cohorts.itertuples()):
-            ds_cohort = ds.isel(cohorts=cohort_index)
+        for cohort in df_cohorts.itertuples():
+            ds_cohort = ds.isel(cohorts=cohort.Index)
             df = pd.DataFrame(
                 {
                     "taxon": cohort.taxon,

malariagen_data/anoph/g123.py

@@ -153,7 +153,7 @@ def g123_gwss(
         self,
         contig: base_params.contig,
         window_size: g123_params.window_size,
-        sites: g123_params.sites = base_params.DEFAULT,
+        sites: g123_params.sites = g123_params.DEFAULT_SITE_PARAMETER,
         site_mask: Optional[base_params.site_mask] = base_params.DEFAULT,
         sample_sets: Optional[base_params.sample_sets] = None,
         sample_query: Optional[base_params.sample_query] = None,
@@ -172,9 +172,6 @@ def g123_gwss(
         # invalidate any previously cached data.
         name = "g123_gwss_v1"
 
-        if sites == base_params.DEFAULT:
-            assert self._default_phasing_analysis is not None
-            sites = self._default_phasing_analysis
         valid_sites = self.phasing_analysis_ids + ("all", "segregating")
         if sites not in valid_sites:
             raise ValueError(
@@ -259,7 +256,7 @@ def _g123_calibration(
     def g123_calibration(
         self,
         contig: base_params.contig,
-        sites: g123_params.sites = base_params.DEFAULT,
+        sites: g123_params.sites = g123_params.DEFAULT_SITE_PARAMETER,
         site_mask: Optional[base_params.site_mask] = base_params.DEFAULT,
         sample_query: Optional[base_params.sample_query] = None,
         sample_query_options: Optional[base_params.sample_query_options] = None,
@@ -279,6 +276,12 @@ def g123_calibration(
         # invalidate any previously cached data.
         name = "g123_calibration_v1"
 
+        valid_sites = self.phasing_analysis_ids + ("all", "segregating")
+        if sites not in valid_sites:
+            raise ValueError(
+                f"Invalid value for `sites` parameter, must be one of {valid_sites}."
+            )
+
         params = dict(
             contig=contig,
             sites=sites,
@@ -314,7 +317,7 @@ def plot_g123_gwss_track(
         self,
         contig: base_params.contig,
         window_size: g123_params.window_size,
-        sites: g123_params.sites = base_params.DEFAULT,
+        sites: g123_params.sites = g123_params.DEFAULT_SITE_PARAMETER,
         site_mask: Optional[base_params.site_mask] = base_params.DEFAULT,
         sample_sets: Optional[base_params.sample_sets] = None,
         sample_query: Optional[base_params.sample_query] = None,
@@ -417,7 +420,7 @@ def plot_g123_gwss(
         self,
         contig: base_params.contig,
         window_size: g123_params.window_size,
-        sites: g123_params.sites = base_params.DEFAULT,
+        sites: g123_params.sites = g123_params.DEFAULT_SITE_PARAMETER,
         site_mask: Optional[base_params.site_mask] = base_params.DEFAULT,
         sample_sets: Optional[base_params.sample_sets] = None,
         sample_query: Optional[base_params.sample_query] = None,

malariagen_data/anoph/g123_params.py

@@ -34,6 +34,8 @@
     """,
 ]
 
+DEFAULT_SITE_PARAMETER: sites = "segregating"
+
 min_cohort_size_default: base_params.min_cohort_size = 20
 
 max_cohort_size_default: base_params.max_cohort_size = 50

malariagen_data/anoph/genome_features.py

@@ -552,5 +552,5 @@ def _transcript_to_parent_name(self, transcript):
             return self._gene_name_overrides[parent_id]
         except KeyError:
             rec_parent = df_genome_features.loc[parent_id]
-            # Try to access "Name" attribute, fall back to "ID" if not present.
-            return rec_parent.get("Name", parent_id)
+            # Try to access gene name attribute, fall back to "ID" if not present.
+            return rec_parent.get(self._gff_gene_name_attribute, parent_id)

tests/anoph/conftest.py

@@ -2464,7 +2464,7 @@ def init_config(self):
             "DEFAULT_COHORTS_ANALYSIS": "20250710",
             "DEFAULT_DISCORDANT_READ_CALLS_ANALYSIS": "",
             "SITE_MASK_IDS": ["dirus"],
-            "PHASING_ANALYSIS_IDS": ["dirus_noneyet"],
+            "PHASING_ANALYSIS_IDS": [],
         }
         config_path = self.bucket_path / "v1.0-config.json"
         with config_path.open(mode="w") as f:
@@ -2786,7 +2786,7 @@ def init_config(self):
             "DEFAULT_COHORTS_ANALYSIS": "20251019",
             "DEFAULT_DISCORDANT_READ_CALLS_ANALYSIS": "",
             "SITE_MASK_IDS": ["minimus"],
-            "PHASING_ANALYSIS_IDS": ["minimus_noneyet"],
+            "PHASING_ANALYSIS_IDS": [],
         }
         config_path = self.bucket_path / "v1.0-config.json"
         with config_path.open(mode="w") as f:

tests/anoph/test_g123.py

@@ -6,6 +6,8 @@
 
 from malariagen_data import af1 as _af1
 from malariagen_data import ag3 as _ag3
+from malariagen_data import adir1 as _adir1
+from malariagen_data import amin1 as _amin1
 from malariagen_data.anoph.g123 import AnophelesG123Analysis
 
 
@@ -56,6 +58,44 @@ def af1_sim_api(af1_sim_fixture):
     )
 
 
+@pytest.fixture
+def adir1_sim_api(adir1_sim_fixture):
+    return AnophelesG123Analysis(
+        url=adir1_sim_fixture.url,
+        public_url=adir1_sim_fixture.url,
+        config_path=_adir1.CONFIG_PATH,
+        major_version_number=_adir1.MAJOR_VERSION_NUMBER,
+        major_version_path=_adir1.MAJOR_VERSION_PATH,
+        pre=False,
+        gff_gene_type="protein_coding_gene",
+        gff_gene_name_attribute="Note",
+        gff_default_attributes=("ID", "Parent", "Note", "description"),
+        default_site_mask="dirus",
+        results_cache=adir1_sim_fixture.results_cache_path.as_posix(),
+        taxon_colors=_adir1.TAXON_COLORS,
+        default_phasing_analysis=None,
+    )
+
+
+@pytest.fixture
+def amin1_sim_api(amin1_sim_fixture):
+    return AnophelesG123Analysis(
+        url=amin1_sim_fixture.url,
+        public_url=amin1_sim_fixture.url,
+        config_path=_amin1.CONFIG_PATH,
+        major_version_number=_amin1.MAJOR_VERSION_NUMBER,
+        major_version_path=_amin1.MAJOR_VERSION_PATH,
+        pre=False,
+        gff_gene_type="protein_coding_gene",
+        gff_gene_name_attribute="Note",
+        gff_default_attributes=("ID", "Parent", "Note", "description"),
+        default_site_mask="minimus",
+        results_cache=amin1_sim_fixture.results_cache_path.as_posix(),
+        taxon_colors=_amin1.TAXON_COLORS,
+        default_phasing_analysis=None,
+    )
+
+
 # N.B., here we use pytest_cases to parametrize tests. Each
 # function whose name begins with "case_" defines a set of
 # inputs to the test functions. See the documentation for
@@ -76,6 +116,14 @@ def case_af1_sim(af1_sim_fixture, af1_sim_api):
     return af1_sim_fixture, af1_sim_api
 
 
+def case_adir1_sim(adir1_sim_fixture, adir1_sim_api):
+    return adir1_sim_fixture, adir1_sim_api
+
+
+def case_amin1_sim(amin1_sim_fixture, amin1_sim_api):
+    return amin1_sim_fixture, amin1_sim_api
+
+
 def check_g123_gwss(*, api, g123_params):
     # Run main gwss function under test.
     x, g123 = api.g123_gwss(**g123_params)
@@ -115,6 +163,10 @@ def test_g123_gwss_with_default_sites(fixture, api: AnophelesG123Analysis):
 
 @parametrize_with_cases("fixture,api", cases=".")
 def test_g123_gwss_with_phased_sites(fixture, api: AnophelesG123Analysis):
+    # Skip if this dataset has no phasing analyses (e.g., Adir1, Amin1).
+    if not api.phasing_analysis_ids:
+        pytest.skip("No phasing analyses available for this dataset.")
+
     # Set up test parameters.
     all_sample_sets = api.sample_sets()["sample_set"].to_list()
     g123_params = dict(
@@ -182,6 +234,10 @@ def test_g123_gwss_with_bad_sites(fixture, api: AnophelesG123Analysis):
 
 @parametrize_with_cases("fixture,api", cases=".")
 def test_g123_calibration(fixture, api: AnophelesG123Analysis):
+    # Skip if this dataset has no phasing analyses (e.g., Adir1, Amin1).
+    if not api.phasing_analysis_ids:
+        pytest.skip("No phasing analyses available for this dataset.")
+
     # Set up test parameters.
     all_sample_sets = api.sample_sets()["sample_set"].to_list()
     window_sizes = np.random.randint(100, 500, size=random.randint(2, 5)).tolist()

tests/anoph/test_genome_features.py

@@ -169,7 +169,9 @@ def test_plot_genes_with_gene_labels(fixture, api: AnophelesGenomeFeaturesData):
     # For each contig in the fixture...
     for contig in fixture.contigs:
         # Get the genes for this contig.
-        genes_df = api.genome_features(region=contig).query("type == 'gene'")
+        genes_df = api.genome_features(region=contig).query(
+            f"type == '{api._gff_gene_type}'"
+        )
 
         # If there are no genes, we cannot label them.
         if not genes_df.empty:
@@ -181,7 +183,10 @@ def test_plot_genes_with_gene_labels(fixture, api: AnophelesGenomeFeaturesData):
 
             # Put the random gene "ID" and its "Name" in a dictionary.
             random_gene_labels = dict(
-                zip(random_sample_genes_df["ID"], random_sample_genes_df["Name"])
+                zip(
+                    random_sample_genes_df["ID"],
+                    random_sample_genes_df[api._gff_gene_name_attribute],
+                )
             )
 
             # Check that we get a Bokeh figure from plot_genes() with these gene_labels.