Skip to content

Conversation

@dpopleton
Copy link
Contributor

Here is a long one with many significant changes.

  • Models have been updated to the new schema.
  • All functions have been modified to take the new models
  • Unit testing has been converted over to sqlite
  • Updater can handle assembly accessions with different number of sequences
  • assembly sequences alias added
  • assembly sequence now populates accession correctly
  • genome group support
  • many bug fixes

Sorry for the length.

Includes three scripts for moving txt/sql files to a mysql server and dumping them back as sqlite.
The conftest.py has options for running with the sqlite db
A second commit will follow with deletion of all mysql related testing
Includes three scripts for moving txt/sql files to a mysql server and dumping them back as sqlite.
The conftest.py has options for running with the sqlite db
A second commit will follow with deletion of all mysql related testing
Implemented assembly uuid control
Genome group support
Simplified lookups for data moved to genome table
Alias loading
Assembly accession loading
genebuild.version removed from updater
multiple meta of same type allowed
Checks for single only meta keys
Fix for meta key verification
Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

Partial review, more to follow soon.

Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

Almost there, only 2 files and a half left.

Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

Almost there, only 2 files and a half left.

Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

Almost there! 💪

Comment on lines +138 to +141
self,
genome: Genome,
release_id: int,
release_type: str
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self,
genome: Genome,
release_id: int,
release_type: str
self,
genome: Genome,
release_id: int,
release_type: str

Comment on lines +267 to +268
self,
genome: Genome
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self,
genome: Genome
self,
genome: Genome

Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

A few changes left, but given the size of the PR, great work!

I would also suggest to split src/tests/test_exports.py in 3 files, one with each class, so reduce the amount of lines in a single file.

Copy link
Contributor

@JAlvarezJarreta JAlvarezJarreta left a comment

Choose a reason for hiding this comment

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

A few changes left, but given the size of the PR, great work!

I would also suggest to split src/tests/test_exports.py in 3 files, one with each class, so reduce the amount of lines in a single file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants