rii-mango / NIFTI-Reader-JS

A JavaScript NIfTI file format reader.
MIT License
137 stars 30 forks source link

Massive distributable artifact #11

Closed kmannislands closed 3 years ago

kmannislands commented 3 years ago

We're installing nifti-reader-js via yarn modern where it's common to check in dependencies.

It has come to my attention that of our ~5k transitive dependencies, nifti reader js is the largest by a healthy margin.

Zipped, the library's source is still 24.2MB!

Poking around the repo, the cause is pretty clear:

❯ ls -laS ./tests/data
total 70008
-rw-r--r--   1 kieranmann  staff  33423712 Oct 27 09:10 5D.nii
-rw-r--r--   1 kieranmann  staff    902981 Oct 27 09:10 avg152T1_LR_nifti.nii
-rw-r--r--   1 kieranmann  staff    763810 Oct 27 09:10 avg152T1_LR_nifti2.nii.gz
-rw-r--r--   1 kieranmann  staff    531671 Oct 27 09:10 avg152T1_LR_nifti.nii.gz
-rw-r--r--   1 kieranmann  staff     72939 Oct 27 09:10 big.nii.gz
-rw-r--r--   1 kieranmann  staff     71853 Oct 27 09:10 little.nii.gz
-rw-r--r--   1 kieranmann  staff     58809 Oct 27 09:10 with_extension.nii.gz
-rw-r--r--   1 kieranmann  staff      1024 Oct 27 09:10 not-nifti.nii
drwxr-xr-x  13 kieranmann  staff       416 Oct 27 09:10 ..
drwxr-xr-x  10 kieranmann  staff       320 Oct 27 09:10 .

The 5D nifti test dataset is massive for checked in content, weighing in at over 30 MB.

Would you be open to a PR replacing it with a smaller 5D nifti?

martinezmj-ims commented 3 years ago

Yes, absolutely. A PR to be fix would be welcome.

kmannislands commented 3 years ago

@martinezmj Great, opened.

I would note that a more modern assertion library would be beneficial in your test suite. Noticed that test expectations are rather limited and I think more powerful assertions would help.

Also, unrelated, but would you be open to modernizing the syntax used here? Pains me to write var in 2021 😄