Skip to content

Conversation

@bilalebi
Copy link
Contributor

@bilalebi bilalebi commented Jun 26, 2023

JIRA Ticket

https://www.ebi.ac.uk/panda/jira/browse/EA-1090

Changes

Added the possibility to fetch genome by ensembl_name and assembly_name (genebuild_id will be added in the future)


Update:

Major changes

  • Added fetch genome by ensembl name and assembly name
  • Remove mutual exclusivity in fetch_genomes() and fetch_sequences()
  • Improved fetch_taxonomy_names() to include ncbi_common_name, alternative_names and common_name
  • Improved fetch_sequences() to include assembly_uuid
  • Added fetch_genome_by_keyword()
  • Added and fixed broken tests
  • Improved the overall code logic
  • Added the possibility to fetch unreleased genomes
  • ...

This branch is also used to update the API to be used by ensembl-metadata-service

Related PR:

Ensembl/ensembl-metadata-service#25

@bilalebi bilalebi added the enhancement New feature or request label Jun 26, 2023
@bilalebi bilalebi requested review from dpopleton and vinay-ebi June 26, 2023 15:52
@bilalebi bilalebi self-assigned this Jun 26, 2023
@bilalebi
Copy link
Contributor Author

Thanks @dpopleton for your suggestions, I pushed the required changes

@bilalebi bilalebi requested a review from dpopleton June 27, 2023 13:57
@bilalebi bilalebi requested a review from dpopleton July 4, 2023 11:03
@bilalebi
Copy link
Contributor Author

bilalebi commented Jul 26, 2023

Hi @vinay-ebi
Please feel free to take a look at this PR, some of the major changes:

  • Added fetch genome by ensembl name and assembly name
  • Remove mutual exclusivity in fetch_genomes() and fetch_sequences()
  • Improved fetch_taxonomy_names() to include ncbi_common_name, alternative_names and common_name
  • Improved fetch_sequences() to include assembly_uuid
  • Added fetch_genome_by_keyword()
  • Added and fixed broken tests
  • Improved the overall code logic
  • Added the possibility to fetch unreleased genomes
  • ...

NB: I commented 3 out of 4 tests in the updater because they were failing and they need to be fixed based on the changes @dpopleton made in this PR #32 that can be done later

@vinay-ebi
Copy link
Contributor

Hi @vinay-ebi Please feel free to take a look at this PR, some of the major changes:

  • Added fetch genome by ensembl name and assembly name
  • Remove mutual exclusivity in fetch_genomes() and fetch_sequences()
  • Improved fetch_taxonomy_names() to include ncbi_common_name, alternative_names and common_name
  • Added fetch_genome_by_keyword()
  • Added and fixed broken tests
  • Added __repr__ functions in the models
  • Improved the overall code logic

NB: I commented 3 out of 4 tests in the updater because they were failing and they need to be fixed based on the changes @dpopleton made in this PR #32 that can be done later

Hi @bilalebi , changes look good to me, let @dpopleton have the final look

Copy link
Contributor

@dpopleton dpopleton left a comment

Choose a reason for hiding this comment

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

So a few minor comments. It looks good for the most part. I have not tested it further or checked for conflicts , so I hope you have. :)

@bilalebi bilalebi requested a review from dpopleton September 11, 2023 08:21
@bilalebi bilalebi requested a review from dpopleton September 12, 2023 17:35
@bilalebi bilalebi changed the title Add fetch genome by ensembl name and assembly name Improve code logic and add fetch genome by ensembl and assembly name Sep 13, 2023
@marcoooo marcoooo dismissed dpopleton’s stale review September 13, 2023 10:45

All recommendations implemented as requested

@marcoooo marcoooo merged commit 2691b57 into main Sep 13, 2023
@dpopleton dpopleton deleted the feat/ftch_gnome_by_assemb_nme_gn_bld_id branch January 17, 2024 10:22
marcoooo added a commit that referenced this pull request Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants