search

lina-usc / pylossless

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

Enforce PEP8 in pipeline.py #48

Closed scott-huberty closed 1 year ago

scott-huberty commented 1 year ago

We need to clean up this module, enforce and adhere to PEP8 guidance. This is good practice for obvious reasons and will make it easier for us to spot errors in our respective code editors.

But BEFORE we do this we need to finish #39 because we should really try to limit PR's until we have a system in place to mitigate code regression.

Andesha commented 1 year ago

I don't mind taking ownership of this. It can really help me get familiar with the code if you don't mind me working on it.

In places where I don't know what a function does or can't figure it out I can add something like a TODO...

scott-huberty commented 1 year ago

@Andesha That would be awesome!

We at least have one test in our CI pipeline, and hopefully a couple more will be in main soon, which makes me less worried about opening a new PR for this ticket.

it'll be good to clean up our flakes so that I can add a flake test to the CI.

Andesha commented 1 year ago

Just wrapped up my first pass on this...

I believe it's viewable here.

You've done a lot of work and a really great job :smile:

I have some feedback about some of the config structures as well. I sent you an email about some more casual conversation methods as well :+1:

scott-huberty commented 1 year ago

Just wrapped up my first pass on this...

Awesome, thank you @Andesha !! Maybe when you are ready to open a PR we can add a flake test (even if just on a subset of the modules) to the CI, and close off the PR with that (no rush).

We are Happy to hear any suggestions on the Config (and on the repo generally). I'm sure there is a lot that can be improved!