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

Tarball size is very large for project size #103

Open
IAlibay opened this issue Jan 4, 2023 · 6 comments
Open

Tarball size is very large for project size #103

IAlibay opened this issue Jan 4, 2023 · 6 comments

Comments

@IAlibay
Copy link
Member

IAlibay commented Jan 4, 2023

The pytng tarball is currently larger than the MDAnalysis tarball size (by roughly 3x).

The main source of this is the inclusion of test files as part of the tarball. I'm opening this issue to see if this is required / can be addressed in some way.

@tylerjereddy
Copy link
Member

Test files as in data files used as assets for testing? I know SciPy just went through a transition to use pooch for some dataset handling, not sure if appropriate for some parts of MDA.

@IAlibay
Copy link
Member Author

IAlibay commented Jan 4, 2023

Test files as in data files used as assets for testing?
Yeah, essentially anything that's under https://github.com/MDAnalysis/pytng/tree/master/tests.

pooch seems interesting, were there any concerns by the scipy team about needing external internet access to get extra files? (rather than I guess just passing the tarball up to whichever hpc facility needs it)

@richardjgowers
Copy link
Member

looking at the test files, there's just one traj which is large (8mb). (This is an actual TNG file from mdrun, which I think is valuable for development as gmx doesn't actually follow the public API in how it creates TNG files...)

I think we could probably exclude this file from the tarball and run most but not all tests on a local installation.

We could probably also not include any tests in the tarball, I'm not sure what the done thing is and if this is too aggressive.

@tylerjereddy
Copy link
Member

fwiw, this is the discussion that happened: scipy/scipy#15607

@IAlibay
Copy link
Member Author

IAlibay commented Jan 4, 2023

We could probably also not include any tests in the tarball, I'm not sure what the done thing is and if this is too aggressive.

For pyedr we just lowered the file sizes to a reasonable limit and shipped that. I think the main thing would be whether or not you're expecting users to run tests on pytng post installation (i.e. if it's documented and/or you have a means of importing the datafiles that's user facing). If not you can probably just not include them or add them to MDAanlysisTests or MDAnalysisData instead?

@richardjgowers
Copy link
Member

I don't think we expose running tests from an installation, so we could not include any in the tarball for now and cut a more svelte build without any drama. Then once/if we include some installation smoketests we can reinclude these.

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

No branches or pull requests

3 participants