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

feat: migrate to pyproject.toml for building package #6880

Merged
merged 28 commits into from
Mar 29, 2023
Merged

feat: migrate to pyproject.toml for building package #6880

merged 28 commits into from
Mar 29, 2023

Conversation

SauravMaheshkar
Copy link
Contributor

Changes proposed by this PR can be summarized as follows :-

  • Delete setup.py in favour of pyproject.toml for packaging.
  • Update pre-commit configuration with an updated version of pyroma to check packaging quality of pyproject.toml.
  • Update workflows to build using python -m build.

pyproject.toml is the new standard for packaging python packages, setup.py is now deprecated (first introduced in PEP 518 and later expanded in PEP 517, PEP 621 and PEP 660).

@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #6880 (0446028) into master (98a4d72) will not change coverage.
The diff coverage is n/a.

❗ Current head 0446028 differs from pull request most recent head e82c9f2. Consider uploading reports for the commit e82c9f2 to get more accurate results

@@           Coverage Diff           @@
##           master    #6880   +/-   ##
=======================================
  Coverage   91.27%   91.27%           
=======================================
  Files         436      436           
  Lines       23891    23891           
=======================================
  Hits        21806    21806           
  Misses       2085     2085           

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

@SauravMaheshkar
Copy link
Contributor Author

@rusty1s, the pre-commit checks are passing on my machine with the updated version of pyroma. Not sure why it is failing in CI.

@SauravMaheshkar
Copy link
Contributor Author

@rusty1s I think the pre-commit CI is failing because it picks up the configuration from the master branch and not the branch the PR is from. I think you'll be able to verify that pre-commit passes by running it on my branch using the updated pre-commit configuration with the newer version of pyroma (the version that supports PEP 517).

@rusty1s
Copy link
Member

rusty1s commented Mar 15, 2023

Got it. Will try to get this in as part of our PyG 2.3 release.

@SauravMaheshkar
Copy link
Contributor Author

@rusty1s do you think this will get into the release (hopefully) happening on March 21 👀

@rusty1s
Copy link
Member

rusty1s commented Mar 20, 2023

Likely not, I would like to delay this to next week to guarantee that PyG 2.3 release goes smooth.

@SauravMaheshkar
Copy link
Contributor Author

Likely not, I would like to delay this to next week to guarantee that PyG 2.3 release goes smooth.

Ahh of course, makes sense 👍🏽

@SauravMaheshkar
Copy link
Contributor Author

SauravMaheshkar commented Mar 23, 2023

@rusty1s Seems like there is a conflict in setup.py because of the version bump which is not triggering the CI.

@SauravMaheshkar
Copy link
Contributor Author

@rusty1s can you fix this merge conflict ? I'm not sure if simply bumping the version in setup.py would help.

@rusty1s rusty1s enabled auto-merge (squash) March 29, 2023 14:23
@SauravMaheshkar
Copy link
Contributor Author

SauravMaheshkar commented Mar 29, 2023

Interesting, it's taking forever to finish. They all finished successfully idk why it's stuck though. I'd recommend cancelling and then restarting seems like a waste of compute right now with CI taking 60 mins.

Nvm, seems like Github Actions are down.

@rusty1s rusty1s merged commit 80d4647 into pyg-team:master Mar 29, 2023
@rusty1s
Copy link
Member

rusty1s commented Mar 29, 2023

It's working again:D @SauravMaheshkar Disabled pyroma for now, but maybe we can dig into the root cause of the failure. Hopefully, we can get it back one day :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants