fossasia / pslab-python

Python Library for PSLab Desktop: https://pslab.io
GNU General Public License v3.0
1.63k stars 225 forks source link

Split up sciencelab.py into smaller modules #127

Closed bessman closed 3 years ago

bessman commented 4 years ago

The Sciencelab class is currently almost 4000 lines long. It is actually so long that Codacy ignores it when checking code quality.

As a rule of thumb, I like to keep classes under 500 lines or so. I intend to split the Sciencelab class into smaller classes based on individual instruments, like Multimeter, Oscilloscope, Waveform, LogicAnalyzer, etc. Possibly with some parent base class to avoid duplication of shared functionality.

19-pragya commented 4 years ago

can i work on this?

bessman commented 4 years ago

Sure. Like I said on #102, any module except for the logic analyzer is up for grabs.

vaibhavarora102 commented 4 years ago

@bessman
I am a participant for Codeheat can do work on this?

just to clarify: we had to divide the class into smaller classes and then again access them by some parent class? am I right? @mariobehling

vaibhavarora102 commented 4 years ago

It's having 1732 lines Is their any change?

bessman commented 4 years ago

Hi @vaibhavarora102! Of course, you are welcome to work on this 😃.

I've already refactored out the oscilloscope and logic analyzer, which were about a thousand lines each. Remaining modules which should be refactored out are:

Refactoring a module includes the following tasks:

Take a look at #129 and #132 for examples.

vaibhavarora102 commented 4 years ago

@bessman should I continue with wave form that would be done by you Thanks

bessman commented 4 years ago

I'll keep working on the waveform generator. You can go ahead with either the multimeter or the power supply. Of the two, the multimeter is significantly more complicated, so it is probably a good idea for you to start with the power supply.

vaibhavarora102 commented 4 years ago

hi @bessman
can you please clarify that in power supply their comes: 1) Motor signalling 2) READ PROGRAM AND DATA ADDRESSES 3) ANALOG OUTPUTS

or any thing else also? or i any thing less?

thanks

bessman commented 4 years ago

The power supply functionality consists of the methods set_pv1, set_pv2, set_pv3, set_pcs, and the associated get_* methods. L1292 until L1372 in the current version of sciencelab.py. These methods depend on the DACCHAN and MCP4728 classes from the Peripherals module.

These methods and classes should be moved to a new module, power_supply. When moving the code, refactor it to comply with the style guide. Enable linting of the new file by adding it to the INCLUDE_PSL_FILES environment variable of tox.ini. Finally, write integration tests and place recordings of passing tests in tests/recordings/power_supply.

vaibhavarora102 commented 4 years ago

@bessman sir kindly unassigne the issue As I had already wasted much time, and still is quiet confused over this issue Thanks

bessman commented 4 years ago

Hi @vaibhavarora102, do you mean that you no longer wish to work on this issue, or that you would like to be assigned to it? I forgot to assign you earlier, sorry about that.

If you still want to work on this, I can try to answer any question you might have. Otherwise, no worries, thanks for giving it a try!

vaibhavarora102 commented 4 years ago

Sure if it means that I could ask as many queries as I have, then kindly assign and will work progressively

bessman commented 4 years ago

Of course, ask away and I will do my best to answer.

bessman commented 4 years ago

Hi @vaibhavarora102, are you still working on this?

vaibhavarora102 commented 4 years ago

Yes am working will create pull request for initial code Within 2-3 days

vaibhavarora102 commented 4 years ago

@bessman thanks for such patience actually there was some health issue but am fine now and will now proceed with full motivation, some speed

had made initial commits and pull request kindly check if they are fine and guide for further proceeding I assure you to be quick this time thanks

Skynet843 commented 4 years ago

can i do this work?

MohitPatni0731 commented 3 years ago

Can I work on this issue

mansijain980 commented 3 years ago

@bessman sir, may i start to refactor "Power supply " module ? I am a participant in Codeheat . I can make initial split for power supply. I got how to divide sciencelab.py module into smaller classes.

Please assign this issue to me. It will be a nice start for me. Thanks

bessman commented 3 years ago

@mansijain980 Thanks for the offer, but I am already working on it.

mansijain980 commented 3 years ago

@bessman sir, I already started working on that but it's fine. I would like to work on "Multimeter" module then.

Can you assign this to me ?

bessman commented 3 years ago

The multimeter refactorization is done. Feel free to open a pull request with the changes that you have so far, perhaps we can incorporate them somehow.

SAP-20 commented 3 years ago

A newbie in the open source contributing.Sir,Could you assign me this project to me??

bessman commented 3 years ago

Hi @SAP-20. We are moving away from assigning issues, and instead encourage contributors to work on whatever interests them, and to open pull requests whenever they are ready to discuss their changes.

This particular issue is just about finished, as of #158. If any of the other open issues seem interesting to you, please go ahead and work on any of them you'd like. No need to wait to be assigned :smile:

One of the most valuable ways to contribute to pslab-python right now is to simply use the library. We now have automated tests for most of it, but automated tests can't compete with actual users when it comes to finding bugs. Right now, bug reports are probably more valuable to us than code contributions.

bessman commented 3 years ago

Most of the useful stuff in sciencelab.py has now been moved to separate modules. It is over 3000 lines of code shorter than it was when this issue was opened. Most of what remains in sciencelab.py is unused and/or broken and should be removed.