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

Remove tests from package #1611

Merged
merged 2 commits into from
May 28, 2024
Merged

Conversation

cachitas
Copy link
Contributor

Once installed, the package got ~11x bigger with the release of 2024.5.0.

I see that you are now shipping files under tests folders that were not included before. Going through the latest commits, I couldn't tell if this was on purpose or just overlooked with the transition to the hatch build system.

In this PR I set an exclude rule to mimic the state of the distributed packages as they were in version 2024.3.1.

I am available to discuss this further or make any adjustments.

@martindurant
Copy link
Member

I am regretting trying out hatch. Too many things to take care of. Once you have the template, it's all the same, but still...

@martindurant
Copy link
Member

(I see the wheel package size of 172kiB for 2024.3.1 and 316kiB for 2024.5.0, where does "11x" come from?)

@cachitas
Copy link
Contributor Author

Indeed wheel sizes are the ones you mention. I was referring to the size the package takes in site-packages once it is installed: it went from ~600kB to ~8MB (can't recall the numbers exactly).

The main contributors are two test cassettes inside the implementations directory. example

@martindurant
Copy link
Member

I'm not sure we care about installed size, but I'm happy to hear otherwise. However, it does seem reasonable to keep the same behaviour as pre-hatch. I'll leave this open a couple of days to see if there are more opinions.

@cachitas
Copy link
Contributor Author

I agree, the size shouldn't be an issue nowadays. I just mentioned it because it was what triggered me into looking more closely to this new release, wondering what could be going on to justify such a difference.

@martindurant martindurant merged commit 4f8c4c5 into fsspec:master May 28, 2024
11 checks passed
@cachitas cachitas deleted the dist-contents branch May 31, 2024 12:57
@mgorny
Copy link
Contributor

mgorny commented Jun 4, 2024

While I don't mind removing them from wheels/installed files, Linux distributions would really appreciate tests back in sdist, since we actually run them.

@martindurant
Copy link
Member

Some seem to test them anyway: #1572

Actually, the trend seems to have been to move tests out of the source directory altogether.

How would one include tests in one release type and not the other?

@mgorny
Copy link
Contributor

mgorny commented Jun 4, 2024

I suppose the simplest way would be to move tests out of package directory, e.g. into the top-level test directory inside the git repo. Then you wouldn't have to exclude parts of the package to avoid installing them.

@cachitas
Copy link
Contributor Author

cachitas commented Jun 4, 2024

Note that in this PR I ended up not removing all test files, I just applied the exclusions that were in place before migrating to hatch.

How would one include tests in one release type and not the other?

Here, I applied the same rule to both sdist and wheel, but you can specify different rules for different targets (see example from hatch documentation).

[tool.hatch.build.targets.wheel]
...

[tool.hatch.build.targets.sdist]
...

This way is possible to exclude all test related files from the wheel and keep some/all in sdist.

@martindurant
Copy link
Member

OK, well I'd be happy to see further changes to satisfy package consumers, but I have no firm stance on any of this.

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.

None yet

3 participants