💚 Add pylint in CI #39

[tkoyama010]

See #39

[codecov]

Codecov Report

Merging #40 into master will increase coverage by 2.92%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   95.00%   97.92%   +2.92%     
==========================================
  Files           5        5              
  Lines         440      434       -6     
==========================================
+ Hits          418      425       +7     
+ Misses         22        9      -13     

[GuillaumeFavelier]

Do you need help with this @tkoyama010 ? I could push to the PR if you want.

[tkoyama010]

@GuillaumeFavelier It will take time to resolve the error, so I appreciate your help. Thanks ☺

[GuillaumeFavelier]

Locally it's 8.87/10 but on Github Actions it's actually 7.40/10. One way to fix this is probably to install the requirements_test.txt.

[GuillaumeFavelier]

I set up python correctly with all dependencies but apparently python-lint is built with docker so it did not change anything.

In this context, I have no solution to fix:

pyvistaqt/plotting.py:47:0: E0401: Unable to import 'numpy' (import-error)
pyvistaqt/plotting.py:48:0: E0401: Unable to import 'vtk' (import-error)
pyvistaqt/plotting.py:50:0: E0401: Unable to import 'scooby' (import-error)
pyvistaqt/plotting.py:52:0: E0401: Unable to import 'vtk.qt.QVTKRenderWindowInteractor' (import-error)
pyvistaqt/plotting.py:53:0: E0401: Unable to import 'PyQt5' (import-error)
pyvistaqt/plotting.py:54:0: E0401: Unable to import 'PyQt5.QtCore' (import-error)
pyvistaqt/plotting.py:55:0: E0401: Unable to import 'PyQt5.QtWidgets' (import-error)
pyvistaqt/plotting.py:69:0: E0401: Unable to import 'pyvista' (import-error)
pyvistaqt/plotting.py:70:0: E0401: Unable to import 'pyvista.utilities' (import-error)
pyvistaqt/plotting.py:71:0: E0401: Unable to import 'pyvista.plotting.plotting' (import-error)
pyvistaqt/plotting.py:72:0: E0401: Unable to import 'pyvista.plotting.theme' (import-error)

If someone has an idea, please chime in. Meanwhile, I'll restore the workflow how it was.

[tkoyama010]

Pylint said that plotting.py code is too big (C0302).
https://github.com/pyvista/pyvistaqt/runs/923356349?check_suite_focus=true#step:4:14

According to this diagram. We should split this to BackgroundPlotter and MainWindow. How do @GuillaumeFavelier @akaszynski
@banesullivan @larsoner think?

pyvistaqt/pyvistaqt/plotting.py

Lines 1 to 14 in 49ac22b

	"""
	Diagram
	^^^^^^^
	.. code-block:: none
	BackgroundPlotter
	+-- QtInteractor
	|-- QVTKRenderWindowInteractor
	| +-- QWidget
	+-- BasePlotter
	MainWindow
	+-- QMainWindow

[GuillaumeFavelier]

BackgroundPlotter depends on MainWindow but not the other way around. We could put the MainWindow class into a window.py file? Same idea for Counter in counter.py.

ScaleAxesDialog and FileDialog could go into dialog.py.
DoubleSlider and RangeGroup could be moved too I think.

[tkoyama010]

Thak you for reading this PR 🙇 Since it is currently difficult to satisfy all of the pylint criteria, we may want to determine which errors and warnings to ignore.

[banesullivan]

I'm in favor of @GuillaumeFavelier's suggestions for splitting up the modules

[GuillaumeFavelier]

@tkoyama010 I let you resolve the conflicts the way you prefer :)

[tkoyama010]

As we know pylint is the hardest code checker. Let's resolve pycodestyle in #45 and flake8 in #46 first 🚀 It will also reduce the error of pylint too.

[GuillaumeFavelier]

@tkoyama010 this is fairly big contribution, I may take some time to review this one.

Meanwhile, I detected a regression in an IPython environment. The following is still needed for example:

from PyQt5 import QtCore, QtGui
from PyQt5.QtCore import QTimer, pyqtSignal
from PyQt5.QtWidgets import QAction, QFrame, QMenuBar, QVBoxLayout

I will commit to restore this plus some minor changes.

[tkoyama010]

@GuillaumeFavelier Thanks for your review. I am sorry that I could not find erorr of ipython. Could we add Ipython test and check it in CI? (It maybe another issue.)

[GuillaumeFavelier]

Could we add Ipython test and check it in CI? (It maybe another issue.)

+100 for this, in another PR though :)

[GuillaumeFavelier]

IMO, this is good to go 👍

What about the other @pyvista/developers ?

[akaszynski]

I see no issues with this. Let me know if you're good with merging this.

[tkoyama010]

Thanks @akaszynski I am ready for merge 🚀