ticket:PSB-19: Fixup typing #2642
Conversation
| @@ -123,6 +123,10 @@ def postprocess_additional(self): | |||
| self._df['date_of_acquisition_behavior'] | |||
| self._df = self._df.drop(['date_of_acquisition_behavior', | |||
| 'date_of_acquisition_ophys'], axis=1) | |||
| # Enforce an integer type on ophys_session_id due to many | |||
| # columns being NaN. | |||
| self._df['ophys_session_id'] = self._df['ophys_session_id'].astype( | |||
There was a problem hiding this comment.
Not sure I understand the comment here. Isn't it returned as int from the database?
There was a problem hiding this comment.
I'll clarify. This ends up being a pandas issue as you are merging in a column that doesn't have values for all the behavior_sessions.
There was a problem hiding this comment.
Did not know about the Int64 dtype. Thanks
| @@ -9,17 +9,19 @@ | |||
| class MouseId(DataObject, LimsReadableInterface, JsonReadableInterface, | |||
| NwbReadableInterface): | |||
| """the LabTracks ID""" | |||
| def __init__(self, mouse_id: int): | |||
| def __init__(self, mouse_id: str): | |||
| if not isinstance(mouse_id, str): | |||
There was a problem hiding this comment.
This goes against the type. It expects string, so if it is not string, then the caller is passing the wrong type and it should be corrected there. Or the type needs to be updated.
There was a problem hiding this comment.
Mostly this was to enforce the type and avoid unittest failures from elsewhere, specifically, from_json calls on ecephys tests. I'll remove the line and see if I can find where the typing is going wrong there specifically.
There was a problem hiding this comment.
Actually, hold on, would you be cool if I moved this type check to the from_json method?
| super().__init__(name="mouse_id", value=mouse_id) | ||
|
|
||
| @classmethod | ||
| def from_json(cls, dict_repr: dict) -> "MouseId": | ||
| mouse_id = dict_repr['external_specimen_name'] | ||
| mouse_id = int(mouse_id) | ||
| mouse_id = mouse_id |
There was a problem hiding this comment.
this line needs to be removed
| return cls(mouse_id=mouse_id) | ||
|
|
||
| @classmethod | ||
| def from_lims(cls, behavior_session_id: int, | ||
| def from_lims(cls, behavior_session_id: str, |
There was a problem hiding this comment.
Sure about this? BehaviorSessionId.value is an int.
There was a problem hiding this comment.
Oops, sorry, trigger happy replace all. thanks for the catch.
morriscb
force-pushed
the
ticket/PSB-19/dev
branch
from
243ee22
to
7d3889d
Compare
No description provided.