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 a few undistributed packages #3447

Merged
merged 9 commits into from
Jan 11, 2025

Conversation

d-torrance
Copy link
Member

This fixes some of the low-hanging fruit from #3446 to get some of the undistributed packages to load.

@d-torrance
Copy link
Member Author

I added a couple more related commits:

  • Fix the html validation bug mentioned in #3448 (comment).
  • Remove the supplanted-packages directory. It just contained old versions of packages that still exist in the main packages directory. That's not necessary using git -- we can just check out an old commit if we want to look at an older version of a package.

@d-torrance d-torrance force-pushed the undistributed-pkgs branch 2 times, most recently from 141bf85 to 008c9d9 Compare September 6, 2024 10:29
@mikestillman
Copy link
Member

@d-torrance OK, I'm finally going to make an attack on PR's and issues. But I am having trouble with this one, for some reason. Besides having deleted files (which are likely fine), I am having trouble with libomp not being found. That works for me now in development, but not this branch (which I though I had merged into development...). Not sure yet what the problem is...

Made a variable local and remove a redundant documentation key.
Remove empty Example lists from documentation
Pass a list of strings (not symbols) to "export".
Remove empty Example list from documentation
The current versions of these packages still exist in the main
packages directory, and these older versions still exist in the git
history.
@d-torrance
Copy link
Member Author

@mikestillman - I've just rebased it onto development, so maybe that will help. Thanks for taking a look!

Copy link
Member

@mikestillman mikestillman left a comment

Choose a reason for hiding this comment

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

Thanks Doug for doing this! I think this is ready to go, but I'd like to see what you say about a question I had in the review. Once you answer those (in any way, really!), lets merge this PR.

@@ -209,7 +209,7 @@ jobs:
make validate-html
else
# otherwise, just validate HTML docs for packages that have been updated
PACKAGES=$(git diff --stat origin/development HEAD -- ../../Macaulay2/packages/ | grep -Po "(?<=Macaulay2/packages/)[^/\.]*(?=\.m2|/)" | uniq | xargs)
Copy link
Member

Choose a reason for hiding this comment

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

What is this doing? Is it now going to try to install these packages?

Copy link
Member Author

Choose a reason for hiding this comment

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

No -- for every pull request, we do a git diff to see if any packages were updated, and if so, validate the html of these packages' docs. In particular, we look for any changes to an .m2 file of M2/Macaulay2/packages or something in a subdirectory of M2/Macaulay2/packages to generate a list of packages to check.

However, undistributed-packages is a subdirectory of M2/Macaulay2/packages but it isn't a package! So we were trying to validate the html docs of a nonexistent package, leading to occasional failing GitHub builds. So now we filter this case out.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, thanks, that sounds good!

@@ -67,7 +67,6 @@ Headline
an interface to the Risa/Asir website and system
Copy link
Member

Choose a reason for hiding this comment

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

This package I would like to flesh out, as Risa/Asir has a few very good algorithms

time syzM = substitute(syz gens M, ring family);
time eq = compress((gens family * syzM) % G);
time G := forceGB gens family;
time syzM := substitute(syz gens M, ring family);
Copy link
Member

Choose a reason for hiding this comment

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

This package is perhaps superseded by GroebnerStrata.m2, but in fact there might be some things here that I want to include there. I had forgotten that this package was here.

@mikestillman
Copy link
Member

Feel free to merge it. Do you tend to do merge PR's or rebase PR's?

@d-torrance
Copy link
Member Author

I usually rebase since that gives us a cleaner looking git history without a bunch of merge commits.

I'm assuming all the tests will pass, but I'll hold off on merging until they do just in case.

@mikestillman
Copy link
Member

Sounds good!

@d-torrance d-torrance merged commit 351b378 into Macaulay2:development Jan 11, 2025
5 checks passed
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