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

Add MANIFEST.in for source distribution #1

Merged
merged 3 commits into from
Mar 16, 2021
Merged

Add MANIFEST.in for source distribution #1

merged 3 commits into from
Mar 16, 2021

Conversation

mrakitin
Copy link
Member

@mrakitin mrakitin commented Feb 24, 2021

This PR adds the MANIFEST.in file required to add necessary files into the source distribution uploaded to PyPI (those files are bundled in the .whl file via setup.py). It is based on https://github.com/NSLS-II/scientific-python-cookiecutter/blob/master/%7B%7B%20cookiecutter.repo_name%20%7D%7D/MANIFEST.in.

Comparison:

main branch:

$ tree lixtools-2021.2.21.1
lixtools-2021.2.21.1
├── PKG-INFO
├── README.md
├── lixtools
│   ├── __init__.py
│   ├── atsas.py
│   ├── hdf.py
│   ├── mailin.py
│   ├── modeling.py
│   └── notebooks.py
├── lixtools.egg-info
│   ├── PKG-INFO
│   ├── SOURCES.txt
│   ├── dependency_links.txt
│   ├── requires.txt
│   └── top_level.txt
├── setup.cfg
└── setup.py

this PR branch:

$ tree lixtools-2021.2.21.1
lixtools-2021.2.21.1
├── LICENSE
├── MANIFEST.in
├── PKG-INFO
├── README.md
├── lixtools
│   ├── __init__.py
│   ├── atsas.py
│   ├── hdf.py
│   ├── mailin.py
│   ├── modeling.py
│   ├── notebooks.py
│   ├── plate_label_template.html
│   └── template_report.ipynb
├── lixtools.egg-info
│   ├── PKG-INFO
│   ├── SOURCES.txt
│   ├── dependency_links.txt
│   ├── requires.txt
│   └── top_level.txt
├── setup.cfg
└── setup.py

@mrakitin
Copy link
Member Author

I'd recommend using the https://github.com/NSLS-II/scientific-python-cookiecutter project to generate all infrastructure files for this project (docs, CI, etc.) Would you like to give it a try following the detailed instruction in https://nsls-ii.github.io/scientific-python-cookiecutter/preliminaries.html, @lyang11973?

@mrakitin
Copy link
Member Author

I'd recommend using the https://github.com/NSLS-II/scientific-python-cookiecutter project to generate all infrastructure files for this project (docs, CI, etc.) Would you like to give it a try following the detailed instruction in https://nsls-ii.github.io/scientific-python-cookiecutter/preliminaries.html, @lyang11973?

I opened an issue #2 for this request, which should be done via a separate PR. This PR is targeting a fix for the missing files (LICENSE, plate_label_template.html, template_report.ipynb).

Also, I haven't observed any tags for this repo. It would be great to have a tag for each PyPI build. The procedure we follow is documented here: https://nsls-ii.github.io/scientific-python-cookiecutter/publishing-releases.html.

Please merge this PR, cut a new tag, and push to PyPI. Then I'll be able to build a conda package via https://github.com/nsls-ii-forge/lixtools-feedstock. Right now, with the missing files, the package is incomplete.

@lyang11973
Copy link
Member

lyang11973 commented Feb 25, 2021 via email

@mrakitin
Copy link
Member Author

As explained in the description, there are missing files in the source distribution .tar.gz file. These updates from the present PR are needed to fix it for the next release.

@mrakitin
Copy link
Member Author

Hi Maxim, since I'm the only one contributing to this package, I would rather keep it simple for now. The files on PyPI are up to date.

@lyang11973, by "simple", do you mean avoiding tags? I don't think they should be disregarded. How does one know what commit was used for a specific version on PyPI? From the packaging perspective that would be useful, for example, if we wanted to temporarily use the archived files from the GitHub tags instead of the sources on PyPI, to work-around the situation with the missing files in the source distribution on PyPI. Does it sound convincing?

From the DAMA perspective of maintaining the beamline-specific packages, anything that goes in the production conda environments on the experimental floor has to meet minimum requirements, and this PR is attempting to address some of them. Please accept and merge this PR, so that we could provide an expected package during the next deployment.

@mrakitin
Copy link
Member Author

@lyang11973, I think the xlrd dependency should be removed from setup.py, as the package is not maintained for .xlsx files starting from v2. Here is my earlier investigation: NSLS-II-IOS/profile_collection#11 (comment). The openpyxl should be used for that purpose, which you already have in your requirements. I'll update the list here.

@mrakitin
Copy link
Member Author

@lyang11973, I saw another release on PyPI today, which still does not contain these fixes: https://pypi.org/project/lixtools/2021.2.25.0/#files. You can verify it yourself by downloading https://files.pythonhosted.org/packages/c4/a0/c2a5deb94962e3966afc2049c94f52dcca8327fecf4178cf24906a0a0845/lixtools-2021.2.25.0.tar.gz and unpacking it. You will see that the package is incomplete:

$ tree lixtools-2021.2.25.0
lixtools-2021.2.25.0
├── PKG-INFO
├── README.md
├── lixtools
│   ├── __init__.py
│   ├── atsas.py
│   ├── hdf.py
│   ├── mailin.py
│   ├── modeling.py
│   ├── notebooks.py
│   ├── ot2.py
│   └── webcam.py
├── lixtools.egg-info
│   ├── PKG-INFO
│   ├── SOURCES.txt
│   ├── dependency_links.txt
│   ├── requires.txt
│   └── top_level.txt
├── setup.cfg
└── setup.py

The wheel file is OK though, as it's controlled by setup.py, not MANIFEST.in. I have to mention that if the .whl file exists on PyPI, it will be used during the pip installation. However, conda uses .tar.gz sources from PyPI, which is a cause of the problem (current build fails: nsls-ii-forge/lixtools-feedstock#3). Also, people may be interested in the source archive for code inspection, etc., so maintaining a proper package is critical. Please consider merging this update ASAP, so the conda packaging can be done flawlessly, and you get the package during the next deployment.

@mrakitin mrakitin merged commit 22bd341 into main Mar 16, 2021
@mrakitin mrakitin deleted the add-manifest branch March 16, 2021 14:11
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.

2 participants