Skip to content
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

Defer import of matplotlib until needed #3579

Merged
merged 16 commits into from
May 9, 2022
Merged

Conversation

bdraco
Copy link
Contributor

@bdraco bdraco commented Apr 11, 2022

Similar to how cartopy and geoip2 are handled, defer import of matplotlib until needed.

Testing with import scapy.packet

Before:
bdraco 52845 0.0 0.2 409457040 61264 s004 T 10:00AM 0:00.22 /opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/Resources/Python.app/Contents/MacOS/Python

After:
bdraco 52900 0.0 0.1 409195664 46368 s004 T 10:01AM 0:00.17 /opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/Resources/Python.app/Contents/MacOS/Python

fixes #3578

matplotlib has a heavy dep chain

I did manual testing to make sure I could still generate images when matplotlib installed, and verified the warning at -H startup still works when its not

@bdraco bdraco force-pushed the matplotlib branch 2 times, most recently from 67b2c95 to 29cc627 Compare April 11, 2022 20:30
matplotlib has a heavy dep chain

Fixes secdev#3578
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #3579 (8e753ef) into master (285824f) will increase coverage by 35.38%.
The diff coverage is 92.85%.

@@             Coverage Diff             @@
##           master    #3579       +/-   ##
===========================================
+ Coverage   50.80%   86.18%   +35.38%     
===========================================
  Files         171      285      +114     
  Lines       40155    64883    +24728     
===========================================
+ Hits        20399    55922    +35523     
+ Misses      19756     8961    -10795     
Impacted Files Coverage Δ
scapy/libs/test_pyx.py 76.19% <ø> (ø)
scapy/layers/inet.py 67.05% <50.00%> (+44.81%) ⬆️
scapy/plist.py 83.50% <83.33%> (+65.77%) ⬆️
scapy/libs/matplot.py 100.00% <100.00%> (ø)
scapy/packet.py 82.13% <100.00%> (+50.77%) ⬆️
scapy/contrib/automotive/gm/gmlan_scanner.py 84.51% <0.00%> (ø)
scapy/contrib/automotive/obd/pid/pids_60_7F.py 100.00% <0.00%> (ø)
scapy/contrib/automotive/obd/mid/__init__.py 100.00% <0.00%> (ø)
scapy/contrib/scada/__init__.py 100.00% <0.00%> (ø)
scapy/layers/usb.py 71.54% <0.00%> (ø)
... and 245 more

@bdraco bdraco marked this pull request as ready for review April 11, 2022 22:00
polybassa
polybassa previously approved these changes May 2, 2022
@bdraco
Copy link
Contributor Author

bdraco commented May 2, 2022

Thanks !

Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay.

  • Could you move the new file to scapy/libs/ ?
  • Would you mind doing the same for PYX to remove extlib the remaining entirely?

Would be awesome, thanks a lot

@gpotter2 gpotter2 added the cleanup Performs some code clean-up label May 2, 2022
@bdraco bdraco requested a review from gpotter2 May 2, 2022 15:11
@bdraco
Copy link
Contributor Author

bdraco commented May 2, 2022

Retested to make sure I could still generate a plot with a.plot(lambda x:x[1].id)

@bdraco
Copy link
Contributor Author

bdraco commented May 2, 2022

I've tried so many incantations with py27 but haven't been able to get it to patch correctly

@bdraco
Copy link
Contributor Author

bdraco commented May 2, 2022

Should be good now. There was a name collision in the first refactor that py27 couldn't handle since it like has . in its load path.

Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR !

@gpotter2 gpotter2 merged commit 064e0d2 into secdev:master May 9, 2022
bdraco added a commit to bdraco/home-assistant that referenced this pull request Jan 3, 2023
@bdraco bdraco mentioned this pull request Jan 3, 2023
19 tasks
balloob pushed a commit to home-assistant/core that referenced this pull request Jan 3, 2023
rlippmann pushed a commit to rlippmann/core that referenced this pull request Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Performs some code clean-up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

matplotlib is loaded even if scapy is running headless
3 participants