-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BD facs diva support #46
Conversation
…some channels as in FACS Diva
Comandeering to resolve conflicts |
'fake bitmask error': os.path.join(BASE_PATH, 'fake_bitmask_error', | ||
'fcs1_cleaned.lmd'), | ||
'Cytek xP5': os.path.join(BASE_PATH, 'Cytek_xP5', 'Cytek_xP5.fcs'), | ||
'guava muse': os.path.join(BASE_PATH, 'GuavaMuse', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add guava muse
file back to the test suite?
fcsparser/tests/test_fcs_reader.py
Outdated
self.assertListEqual(channel_names, pns_names) | ||
self.assertListEqual(list(channel_meta["$PnS"]), pns_names) | ||
|
||
# self.assertListEqual(channel_names, pnn_names) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# self.assertListEqual(channel_names, pnn_names) | |
# self.assertListEqual(channel_names, pnn_names) |
fcsparser/tests/test_fcs_reader.py
Outdated
self.assertListEqual(list(channel_meta["$PnS"]), pns_names) | ||
|
||
# self.assertListEqual(channel_names, pnn_names) | ||
# self.assertListEqual(list(data.columns.values), pnn_names) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# self.assertListEqual(list(data.columns.values), pnn_names) | |
# self.assertListEqual(list(data.columns.values), pnn_names) |
@CaRniFeXeR thanks for the PR! I added CI to run to make sure unit-test are running as part of the PR. Would you be able to address the failing tests and add back missing tests? |
fcsparser/api.py
Outdated
s_key = '$P{0}S'.format(i) | ||
name_n = "" | ||
name_s = "" | ||
if n_key in text.keys(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a short hand version
name_n = text.get(n_key, "")
name_s = text.get(s_key, "")
fcsparser/api.py
Outdated
|
||
channel_names_n = [] | ||
channel_names_s = [] | ||
for i in self.channel_numbers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sting interpolation using named arguments rather than indexed arguments
for channel_number in self.channel_numbers:
n_key = f'$P{channel_number}N'
s_key = f'$P{channel_number}S'
BD facs diva devices only have channel_name_s set for some but not all channels. If not present for all channels fcsparser fallbacks to the channel_names_n.
This pull request includes two main adaptations:
All adaptations have corresponding unit tests