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

Strip tests from numpy and pandas #150

Merged
merged 2 commits into from
Mar 30, 2022
Merged

Conversation

mcous
Copy link
Contributor

@mcous mcous commented Mar 29, 2022

Overview

This PR attempts to alleviate #144 by encoding something that we've been doing frequently during development into the production builds: strip various tests directories from numpy and pandas prior to creating a system image.

This should save about:

  • Pandas: 7.0M
  • Numpy: 2.5M
  • Total: 9.5M

PR should be rebased and merged to preserve commit messages.

Notes

  • Stripping test files is now a configurable option for both Pandas and Numpy
    • Language: positive - "include test files"
    • Default: no - strip files by default
  • Conventions and settings inspired by other packages that do the same sort of thing
    • package/samba4/Config.in
    • package/python-pygame/Config.in

Copy link
Contributor

@SyntaxColoring SyntaxColoring left a comment

Choose a reason for hiding this comment

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

If we put this fixup in package/, it seems like they will be prone to being accidentally overwritten if/when we update these packages from upstream. Should we leave comments saying that they're purposeful Opentrons additions, meant to work around filesystem space problems in /board/opentrons/ot2?

@sfoster1
Copy link
Member

If we put this fixup in package/, it seems like they will be prone to being accidentally overwritten if/when we update these packages from upstream. Should we leave comments saying that they're purposeful Opentrons additions, meant to work around filesystem space problems in /board/opentrons/ot2?

Honestly we should

  • Make these gated by Config.in flags
  • Try and upstream them

@mcous mcous force-pushed the strip-numpy-pandas-tests branch 2 times, most recently from 119cb42 to db5170a Compare March 30, 2022 14:33
@mcous mcous force-pushed the strip-numpy-pandas-tests branch from db5170a to 33474be Compare March 30, 2022 20:55
@mcous
Copy link
Contributor Author

mcous commented Mar 30, 2022

Results of ./utils/diffconfig

❯ ./utils/diffconfig
+BR2_PACKAGE_PYTHON_NUMPY_TESTS n
+BR2_PACKAGE_PYTHON_PANDAS_TESTS n

@mcous
Copy link
Contributor Author

mcous commented Mar 30, 2022

Results of robot testing. Going to merge to unblock #151

  • numpy: 2.5M
  • pandas: 7.1M

edge

# du -hs numpy
11.7M	numpy
# du -hs pandas
24.4M	pandas

This PR

# du -hs numpy
9.2M	numpy
# du -hs pandas
17.3M	pandas

@mcous mcous merged commit ae41e4a into opentrons-develop Mar 30, 2022
@mcous mcous deleted the strip-numpy-pandas-tests branch March 30, 2022 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants