Skip to content

Add support for diagonal asymptotics in non-smooth cases #12

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

Merged
merged 72 commits into from
Mar 12, 2025
Merged

Conversation

turnip314
Copy link
Collaborator

No description provided.

@behackl behackl self-assigned this Mar 10, 2025
Copy link
Collaborator

@behackl behackl left a comment

Choose a reason for hiding this comment

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

The comments so far are mostly about documentation. I still need to review the "interesting" part of the code in asymptotics.py and whitney.py in detail; more comments following later. Looking good so far!

@behackl behackl changed the title Draft changes for non-smooth stuff Add support for diagonal asymptotics in non-smooth cases Mar 12, 2025
Copy link
Collaborator

@behackl behackl left a comment

Choose a reason for hiding this comment

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

I've pushed some (hopefully) uncontroversial improvements in 7338dbe, 72121a7, 80f5ed8, b0ae16d -- and am now done with this branch.

There are two larger suggestions that I think would make sense to include, namely:

  • refactoring diagonal_asy_smooth and turning it into a private function (-> _diagonal_asy_smooth) while moving the doctests to the new and improved diagonal_asy. I'd be more comfortable to modify the interface some more, which would allow reusing more code from diagonal_asy (like casting r, the rational splitting, ...) -- so effectively, to call _diagonal_asy_smooth a bit later.
  • And second, I feel it would also be a good time to remove the deprecated as_symbolic keyword argument.

If you agree, I'd take care of both of these in follow-up PRs though, as I'd really like to get this merged now.

Pretty cool stuff! 🚀

@behackl behackl merged commit 2894d74 into main Mar 12, 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.

3 participants