Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR restructures the database schema to introduce a many-to-many relationship between runs, lanes, and samples. Previously, samples were directly associated with runs; now samples are associated with lanes, and lanes are associated with runs.
Key changes:
- Introduced a new
Lanemodel to represent individual lanes within a run - Created a many-to-many relationship between
LaneandSampleusing an association table - Restructured the data model so that
data_uriis now stored at the lane level instead of the run level
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| lanes: Mapped[Optional[list["Lane"]]] = relationship( | ||
| secondary=lane_sample_association, backref="samples", back_populates="lanes" |
There was a problem hiding this comment.
The relationship configuration has conflicting directives. Both backref="samples" and back_populates="lanes" are specified, but these are mutually exclusive approaches in SQLAlchemy. When using back_populates, you should not use backref. The back_populates should reference the relationship name on the other side (which should be "lanes" on Sample pointing to Lane), but the backref="samples" is creating a back-reference on Lane. Remove the backref parameter and only use back_populates="lanes" here.
| lane_accession: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) | ||
| lane_number: Mapped[int] | ||
| run_accession: Mapped[int] = mapped_column(ForeignKey("runs.run_accession")) | ||
| data_uri: Mapped[str] = mapped_column(nullable=True) |
There was a problem hiding this comment.
The data_uri field is defined as Mapped[str] but is set to nullable=True. This is inconsistent. If the field can be null, it should be typed as Mapped[Optional[str]] to accurately reflect that it's nullable.
| data_uri: Mapped[str] = mapped_column(nullable=True) | |
| data_uri: Mapped[Optional[str]] = mapped_column(nullable=True) |
| samples: Mapped[Optional[list["Sample"]]] = relationship( | ||
| secondary=lane_sample_association, backref="lanes", back_populates="samples" |
There was a problem hiding this comment.
The relationship configuration has conflicting directives. Both backref="lanes" and back_populates="samples" are specified, but these are mutually exclusive approaches in SQLAlchemy. When using back_populates, you should not use backref. The back_populates should reference the relationship name on the other side (which should be "samples" on Lane pointing to Sample), but the backref="lanes" is creating a back-reference on Sample. Remove the backref parameter and only use back_populates="samples" here.
1 participant
No description provided.