lina-usc / pylossless

🧠 EEG Processing pipeline that annotates continuous data
https://pylossless.readthedocs.io/en/latest/
MIT License
22 stars 8 forks source link

Make `Raw` object accessible as an attribute in `LosslessPipeline` Class? #10

Closed scott-huberty closed 1 year ago

scott-huberty commented 1 year ago

You can't access the raw instance in the LosslessPipeline class, but you can access the flags, ICA, etc.

    def __init__(self, config_fname):
        self.flagged_chs = FlaggedChs()
        self.flagged_epochs = FlaggedEpochs()
        self.flagged_ics = FlaggedICs()
        self.config_fname = config_fname
        self.load_config()
        self.ica1 = None
        self.ica2 = None
        self.ic_labels = None

I wanted to plot the ICA time courses, but realized I needed to supply the raw instance. But I ran with pipeline without saving the file to disk (pipeline.run(bids_path, save=False)), and it dawned on me that the only way to access the raw instance that ran through the pipeline is by saving the file to disk.

Okay fine, I didn't save the file, so I just loaded up the test file in mne as a raw object:

Screen Shot 2022-10-03 at 9 28 55 AM

Ok.... those IC's are all line noise... duh, this raw instance I just loaded is not filtered.

Ok fine, i'll just filter the raw instance that I loaded up before plotting the IC time courses.

Screen Shot 2022-10-03 at 9 31 35 AM

That's better..

Question: should the user be able to access the raw instance that ran through the pipeline, in perhaps LosslessPipeline.raw?? Am I missing something????

christian-oreilly commented 1 year ago

If so, I would name it _current_raw to make it explicit that this is not a property of the pipeline but a temporary state. If you do so, that would help avoid passing raw to almost all the methods of the pipeline. I'd suggest:

    def run(self, bids_path, save=True):
        self._current_raw = mne_bids.read_raw_bids(bids_path)
        self._current_raw._bids_path = bids_path

to make sure that we can always trace the raw to its bids path (and adding self._current_raw = None to the init). I think I would make it a private attribute, although I am still on the fence... I think exposing the raw object can open the door to some problems, so I think making it private is safer. If you do this modification, remember for all methods accessing self._current_raw (which would be almost all methods) to have a if self._current_raw is None: return or something of the sort.

scott-huberty commented 1 year ago

OK - what is your opinion on whether the current_raw accessible as an the attribute of LosslessPipeline class?

scott-huberty commented 1 year ago

I ask becasue I want to make sure I'm not totally off base in my understanding of the purpose of the LosslessPipeline class.