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

Upgrade database_knotinfo to version 2024.10.1 #38822

Merged
merged 6 commits into from
Oct 26, 2024

Conversation

soehms
Copy link
Member

@soehms soehms commented Oct 17, 2024

The current database_knotinfo version 2024.10.1 is no longer compatible with Sage because the syntax for 16 knots for which two braid notations are recorded has changed (see the corresponding Release Notes).

This is fixed by this PR. In addition, two new fields (geometric_type and cosmetic_crossing) added in this version are used to implement corresponding methods (is_hyperbolic and cosmetic_crossing_conjecture_verified) of the interface.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@soehms
Copy link
Member Author

soehms commented Oct 17, 2024

For non-hyperbolic knots, the geometric_type field contains additional information. This is construction information for torus and satellite knots. Maybe we can use this for another new method construction and combine it with information from the pretzel_notation field. Would that make sense?

Copy link

github-actions bot commented Oct 17, 2024

Documentation preview for this PR (built with commit 70adf3a; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@soehms soehms marked this pull request as ready for review October 24, 2024 06:37
@soehms soehms requested a review from tscrim October 24, 2024 06:41
Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Two minor things. Once addressed, positive review.

src/sage/knots/knotinfo.py Outdated Show resolved Hide resolved
src/sage/knots/knotinfo.py Outdated Show resolved Hide resolved
src/sage/knots/knotinfo.py Outdated Show resolved Hide resolved
soehms and others added 2 commits October 24, 2024 18:34
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Thank you. One last little cosmetic (pun intended) thing to fix. Once done, you can set a positive review.

src/sage/knots/knotinfo.py Outdated Show resolved Hide resolved
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
@tscrim
Copy link
Collaborator

tscrim commented Oct 25, 2024

Thank you. LGTM.

@soehms
Copy link
Member Author

soehms commented Oct 25, 2024

Thank you. LGTM.

Thanks too!

@vbraun vbraun merged commit 2af7613 into sagemath:develop Oct 26, 2024
18 of 19 checks passed
@soehms soehms deleted the upgrade_database_knotinfo_2024.10.1 branch October 26, 2024 20:45
vbraun pushed a commit to vbraun/sage that referenced this pull request Nov 14, 2024
…date in build/pkgs

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

This PR just adds the autogenerated package update (produced by `sage
--package update database_knotinfo 2024.10.1`) which was missing in
sagemath#38822.

@tscrim I'm sorry that I missed that in sagemath#38822.


### 📝 Checklon

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38885
Reported by: Sebastian Oehms
Reviewer(s): Travis Scrimshaw
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.

3 participants