search

napari / cookiecutter-napari-plugin

Cookiecutter for napari plugins
BSD 3-Clause "New" or "Revised" License
67 stars 39 forks source link

fix include_package_data in npe2 branch #92

Closed tlambert03 closed 2 years ago

tlambert03 commented 2 years ago

Just noticed that @brisvag's package published with the npe2 cookiecutter is lacking napari.yaml in the wheel https://pypi.org/project/napari-molecule-reader/#files

include_package_data is in the wrong section, this fixes

tlambert03 commented 2 years ago

heads up on this one @nclack and @DragaDoncila... anyone who has used the cookiecutter on the npe2 branch already will have broken plugins when they publish to PyPI. and, since they don't receive these updates after running cookiecutter, they probably won't receive them. not sure if we should make a PSA on zulip? 😅

DragaDoncila commented 2 years ago

...ooops. Yeah I think zulip PSA in general would be the way to go. Also, are we even adding that flag in the migration?

@jamesyan-git this will affect your affinder and zarpaint PRs too so make sure you include that line before merging those.

tlambert03 commented 2 years ago

we might actually consider doing some github grepping and PRs

Also, are we even adding that flag in the migration?

not sure what you mean. this is unique to npe2, wasn't necessary before. only exists in the npe2 branch

DragaDoncila commented 2 years ago

not sure what you mean. this is unique to npe2, wasn't necessary before. only exists in the npe2 branch

Sorry, I was referring to the np2 convert command. It looks like we add options.package_data but not include_package_data?

tlambert03 commented 2 years ago

gotcha, yes, lets fix that too. In fact, I'm not really sure we needed options.package_data at all. i think that include_package_data alone might be sufficient (provided it's in the right place). checking now

tlambert03 commented 2 years ago

yep, include_package_data = True is sufficient

tlambert03 commented 2 years ago

going to merge this and follow up with some things

DragaDoncila commented 2 years ago

yep, include_package_data = True is sufficient

Dumb question but, how does it know that napari.yaml is part of package_data if we don't tell it that via options.package_data

tlambert03 commented 2 years ago

from https://setuptools.pypa.io/en/latest/userguide/datafiles.html

In summary, the three options allow you to:

include_package_data Accept all data files and directories matched by MANIFEST.in.

package_data Specify additional patterns to match files that may or may not be matched by MANIFEST.in or found in source control.

exclude_package_data Specify patterns for data files and directories that should not be included when a package is installed, even if they would otherwise have been included due to the use of the preceding options.

tlambert03 commented 2 years ago

ok, a bit more info. If you don't have a MANIFEST.in file, then it looks like include_package_data=True is enough. If you do have a MANIFEST file, then you will need to either add recursive-include * *.yaml to MANIFEST.in, or you need to add napari.yaml to options.package_data