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

Fix tarball and wheel #295

Merged
merged 3 commits into from
Dec 13, 2022
Merged

Fix tarball and wheel #295

merged 3 commits into from
Dec 13, 2022

Conversation

ocefpaf
Copy link
Contributor

@ocefpaf ocefpaf commented Dec 13, 2022

Comment on lines -60 to -76

distribution:
runs-on: ubuntu-latest
needs: [pytest]

steps:
- uses: actions/checkout@v3
- name: Build distributions
run: |
$CONDA/bin/python -m pip install build
$CONDA/bin/python -m build
- name: Publish a Python distribution to PyPI
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')
uses: pypa/gh-action-pypi-publish@release/v1
with:
user: __token__
password: ${{ secrets.PYPI_API_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@malmans2 let me know if this is OK. I factor it out so we can include sdist and wheels only tests in a new GHA.

run: |
cd dist && python -m pip install oceanspy*.whl
python -m twine check *
python -c "import oceanspy; print(f'oceanspy v{oceanspy.__version__}')"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add more tests here, even run the full test suite, but for now importing is enough b/c it already shows the problem and that it is fixed by this PR.

@@ -1,11 +1,15 @@
include AUTHORS.rst
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is gone.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason behind removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file doesn't exist. Maybe I'm missing something but a manifest check could not find it.

Comment on lines +8 to +13
prune assets
prune docs
prune binder
prune paper
prune sciserver_catalogs/
prune *.egg-info
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should reduce the tarball from ~6MB to 881K.

@@ -36,7 +36,7 @@ testpaths = ["oceanspy/tests"]
addopts = "-v --cov"

[tool.setuptools]
py-modules = ["oceanspy"]
packages = ["oceanspy"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only part of this PR that is "really" needed to fix the issue.

@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Merging #295 (fd2ee9a) into main (7cb16ef) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #295   +/-   ##
=======================================
  Coverage   95.25%   95.25%           
=======================================
  Files           9        9           
  Lines        3898     3898           
  Branches      833      833           
=======================================
  Hits         3713     3713           
  Misses        116      116           
  Partials       69       69           
Flag Coverage Δ
unittests 95.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Mikejmnez
Copy link
Collaborator

Thank you @ocefpaf ! I don't fully understand why the docs didn't build, but once they do we can merge this PR.

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Dec 13, 2022

Thank you @ocefpaf ! I don't fully understand why the docs didn't build, but once they do we can merge this PR.

I don't use readthedocs so I'm clueless on what can I do here to fix it. Their error message seems to be a red herring b/c I can clone the PR using that command.

@Mikejmnez
Copy link
Collaborator

Mikejmnez commented Dec 13, 2022

I don't use readthedocs so I'm clueless on what can I do here to fix it. Their error message seems to be a red herring b/c I can clone the PR using that command.

Yeah, I agree it's a weird error. I can also clone the repository with the failing command.

I'll approve this PR and if there is an issue later with the documentation, we can always fix it afterwards. I really want to get the new version of oceanspy released

@Mikejmnez Mikejmnez merged commit 4273f2a into hainegroup:main Dec 13, 2022
@ocefpaf ocefpaf deleted the fix_tarball branch December 13, 2022 20:01
@malmans2
Copy link
Contributor

Thanks @ocefpaf!

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.

The source distribution tarball is an empty package.
3 participants