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

Replace setup.py with equivalent pyproject.toml #539

Merged
merged 3 commits into from
Aug 8, 2024
Merged

Conversation

mfisher87
Copy link
Member

@mfisher87 mfisher87 commented Aug 6, 2024

Copy link

github-actions bot commented Aug 6, 2024

Binder 👈 Launch a binder notebook on this branch for commit e30bac0

I will automatically update this comment whenever this PR is modified

Binder 👈 Launch a binder notebook on this branch for commit 0c167ac

Binder 👈 Launch a binder notebook on this branch for commit 4650f60

Binder 👈 Launch a binder notebook on this branch for commit 0c74bab

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.27%. Comparing base (0d1e02e) to head (0c74bab).
Report is 49 commits behind head on development.

Additional details and impacted files
@@             Coverage Diff              @@
##           development     #539   +/-   ##
============================================
  Coverage        66.27%   66.27%           
============================================
  Files               36       36           
  Lines             3072     3072           
  Branches           537      537           
============================================
  Hits              2036     2036           
  Misses             948      948           
  Partials            88       88           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Tested locally using python -m pip install -U git+https://github.com/icesat2py/icepyx.git@pyproject and install worked:

Building wheels for collected packages: icepyx
  Building wheel for icepyx (pyproject.toml) ... done
  Created wheel for icepyx: filename=icepyx-1.1.1.dev6+g0c167ac-py3-none-any.whl size=84019 sha256=639020f3dae60c23936d3658e4b0a92a47855f997587f693fe58c6de632b14b4
  Stored in directory: /tmp/pip-ephem-wheel-cache-263bvbb_/wheels/82/71/dc/27dcd9147f97ee37fa865a9c6c34839d9163fb43e99929bc32
Successfully built icepyx
Installing collected packages: icepyx
Successfully installed icepyx-1.1.1.dev6+g0c167ac

Just one other small suggestion below 👇

pyproject.toml Outdated Show resolved Hide resolved
@mfisher87
Copy link
Member Author

Thanks @weiji14 for the review! I'm looking at those UML changes and they look really similar, but shifted around a little bit. Should I just ignore them?

@weiji14
Copy link
Member

weiji14 commented Aug 7, 2024

I'm looking at those UML changes and they look really similar, but shifted around a little bit. Should I just ignore them?

Probably ok to ignore them, I think the UML generator isn't very deterministic... Can check with @JessicaS11?

pyproject.toml Show resolved Hide resolved
@weiji14 weiji14 merged commit 38ebec9 into development Aug 8, 2024
9 checks passed
@weiji14 weiji14 deleted the pyproject branch August 8, 2024 00:31
@weiji14
Copy link
Member

weiji14 commented Aug 8, 2024

Oops, the publish to TestPyPI CI is failing, @mfisher87, could you update this line as well?

sed --in-place "s/node-and-date/no-local-version/g" setup.py

@mfisher87
Copy link
Member Author

Woops! My mistake! #541

license = {file = "LICENSE"}
readme = "README.rst"

requires-python = "~=3.7"
Copy link
Member

Choose a reason for hiding this comment

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

Oh shoot, totally overlooked this line. This should have been >=3.7 instead...

Copy link
Member Author

@mfisher87 mfisher87 Aug 15, 2024

Choose a reason for hiding this comment

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

On it! EDIT: #565

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