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

Migrate from setuptools to hatchling #1384

Merged
merged 2 commits into from
Jan 31, 2023
Merged

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Jan 2, 2023

Base: #1371

Change the build-backend from setuptools to hatchling.

Question: Is there any reason for docs/ to be included in the sdist? It's not in the wheel.

For some reason I'm having trouble including the file aesara/d3viz/html/template.html. I'm not sure yet why it's being excluded.

Thank you for opening a PR!

Here are a few important guidelines and requirements to check before your PR can be merged:

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • pre-commit is installed and set up.
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes, and there are no commits that fix changes introduced by other commits in the same branch/BR.
  • There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

@codecov
Copy link

codecov bot commented Jan 3, 2023

Codecov Report

Merging #1384 (cce4fa5) into main (d789a5f) will decrease coverage by 0.02%.
The diff coverage is n/a.

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1384      +/-   ##
==========================================
- Coverage   74.71%   74.69%   -0.02%     
==========================================
  Files         194      194              
  Lines       49826    49730      -96     
  Branches    10539    10527      -12     
==========================================
- Hits        37226    37145      -81     
+ Misses      10272    10262      -10     
+ Partials     2328     2323       -5     
Impacted Files Coverage Δ
aesara/tensor/special.py 90.64% <0.00%> (-0.27%) ⬇️
aesara/tensor/math.py 90.39% <0.00%> (-0.04%) ⬇️
aesara/tensor/inplace.py 100.00% <0.00%> (ø)
aesara/scalar/math.py 85.29% <0.00%> (+0.29%) ⬆️

@maresb
Copy link
Contributor Author

maresb commented Jan 3, 2023

Ok, I think I got it!!! Excludes are now determined by the root /.gitignore union additional excludes from pyproject.toml. The last few missing files were caused by a few /.gitignore rules that were too broad.

Unlike my previous PRs, once #1371 is merged I want to do a "squash and merge" here, so I'm not curating my individual commits.

@maresb maresb marked this pull request as ready for review January 3, 2023 10:56
@maresb
Copy link
Contributor Author

maresb commented Jan 3, 2023

Rebased on latest changes. All tests were green.

@brandonwillard brandonwillard added the enhancement New feature or request label Jan 3, 2023
@maresb
Copy link
Contributor Author

maresb commented Jan 6, 2023

I made another review pass since I hadn't yet checked the wheel. The bin/ directory was missing from the wheel, so I reincluded it. I'm feeling fairly confident now that I've done due diligence here.

@maresb
Copy link
Contributor Author

maresb commented Jan 6, 2023

[deleted, wrong PR]

@maresb
Copy link
Contributor Author

maresb commented Jan 10, 2023

Still open question: Is there any reason for docs/ to be included in the sdist? It's not in the wheel, and it takes up significant space.

@brandonwillard
Copy link
Member

Still open question: Is there any reason for docs/ to be included in the sdist? It's not in the wheel, and it takes up significant space.

Good question; I'm not aware of one at the moment, so we can probably remove it.

@maresb
Copy link
Contributor Author

maresb commented Jan 17, 2023

Still open question: Is there any reason for docs/ to be included in the sdist? It's not in the wheel, and it takes up significant space.

Good question; I'm not aware of one at the moment, so we can probably remove it.

Great, let's do it in the next PR though. (One thing at a time.)

@maresb maresb requested review from dgerlanc and rlouf January 17, 2023 12:01
@maresb maresb mentioned this pull request Jan 17, 2023
@maresb maresb enabled auto-merge (squash) January 31, 2023 09:01
@maresb
Copy link
Contributor Author

maresb commented Jan 31, 2023

Thanks for the approval!

Interesting... if I update the branch with a rebase (via the GH interface), then it discards your approving review. But with a merge update, it retains your approval. (Since I ultimately want to squash-and-merge, the Git history will remain linear.)

@maresb maresb merged commit 31e6666 into aesara-devs:main Jan 31, 2023
@maresb maresb deleted the hatchling branch February 1, 2023 01:59
@maresb
Copy link
Contributor Author

maresb commented Feb 5, 2023

Addendum

This PR broke nightly builds because it didn't adapt the key names for hatch-vcs. See #1406 for the issue and #1407 for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants