-
Notifications
You must be signed in to change notification settings - Fork 997
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
Exclude unnecssary data from pvlib packaging #2271
Comments
#1056 this has been needed for some time. |
Good initiative. Is it common to exclude the tests from a package? |
scipy and pandas both include their tests in their distribution files (although some pandas maintainers have agreed that it would be better to exclude them, pandas-dev/pandas#30741). I see little value in including the tests in the wheels. If we exclude most data files, then the test functions become even less useful to include. It may make sense to continue including the tests (and data files?) in the sdist distributions, I am not sure. |
Few more thoughts:
|
I've finished classifying the files in #1056's spreadsheet and the result shows the Although I understand facilitating the tests can be a source of easing and validating the working environments, it is a driving bottleneck in the installation process. I would just remove them from wheels due to that.
That would add extra time at running tests and adding/maintaining them. Last alternative does not sound too bad, but it's not a small effort considering the amount of files used in the tests (58) and that an unknown amount of expected results are hardcoded, I can't quantify how many. |
If we remove data files for tests from the distribution, does that mean that |
No since I just wonder if the conda recipe will need to be changed, IDK how conda-forge recipes work in the background: |
But wouldn't the errors raise if the test is run locally instead of using a GH action via a pull request? |
Ah I see, you refer to that if I install pvlib (non editable mode, either from the cloned repo or PyPI), then go to the package folder in Python's
would solve it by not allowing any tests to be run from the distribution installation. I personally support that. Edit: but still on the cloned repo everything should work (it's essentially the same as the workflow), so a contributor shouldn't find any problem running them. |
I understand my own question better. Excluding the test files from the distribution wouldn't affect running tests locally on a fork, since the fork clones the repository rather than installs from the distribution. I don't have a concern here. |
I'd like to develop widgets that can run using WASM, and these need to be relatively fast so as not to annoy the user with a wait time. As long as there is no downsides to the package, I think it's preferred to have it as small as possible.
I don't think the required files have significant trailing zeros (at least not the files I inspected). The big culprit is the LinkeTurbidities.h5 file(15.3 MB), so if we were to do something, it should start with this file.
This predominantly applies to test files, which can just be excluded. |
My strong impression is that the suggestion to remove all tests is being promoted and implemented much too rapidly. Why the furious flurry of activity? |
Order of operations here should be something like this:
Just to clarify for anyone reading this issue: the proposed changes to pvlib's tests is to exclude them only from distribution bundles, i.e. the python files delivered when running |
By the way, a fun estimate: reducing wheel size by 1/3 (as suggested by #2277), multiplied by 14k pvlib downloads per day, eliminates >100GB/day data transfer from PyPI. Maybe not world-changing, but if there is no downside... |
Most of the data in the data folder are only used for testing and not useful for any examples. However, they add up a considerable amount of space (MBs) relative to the pvlib package itself. For applications such as WASM (code running in the browser), all this clutter slows down loading significantly. The data folder is currently 57 MB out of a total of 63 MB.
Of course, there are data files that we want to keep such as the Linke turbidity and CEC database. However, a rough estimation shows that removing unused data files can reduce the pvlib package but half!
Excluding/including data files can be specified in the pyproject.toml file, for example:
This can also be done in the MANIFEST.in file, which is pvlib's current approach, and seems to have finer control.
The pvlib/tests folder can also be excluded, and save 2.5 MB.
The text was updated successfully, but these errors were encountered: