Skip to content

Adapt the KnotInfo interface to new Khovanov polynomial data (merged)#35800

Closed
soehms wants to merge 4 commits intosagemath:developfrom
soehms:khovanov_knotjob_adaption
Closed

Adapt the KnotInfo interface to new Khovanov polynomial data (merged)#35800
soehms wants to merge 4 commits intosagemath:developfrom
soehms:khovanov_knotjob_adaption

Conversation

@soehms
Copy link
Copy Markdown
Member

@soehms soehms commented Jun 20, 2023 •
edited
Loading

CAUTION

Although this PR is not marked as merged it has been merged into 10.1.beta6

Note, that Files changed 0! For more information see #35800 (comment).

📚 Description

A note in the current Khovanov polynomial method of the KnotInfo interface says:

The data used here were calculated with the program KhoHo. They are no longer visible on the website as of October 30, 2022. Instead, data computed with KnotJob are now displayed ... This interface will be adapted to the changes in an upcoming release.

The goal of this PR is the announced adaptation. Accordingly the PR lets Sage point to the current version 2023.6.1 of the Python wrapper of the database which contains the new columns.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • 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 accordingly.

⌛ Dependencies

@soehms soehms marked this pull request as draft June 21, 2023 16:58
@soehms soehms marked this pull request as ready for review June 21, 2023 21:47
@soehms soehms force-pushed the khovanov_knotjob_adaption branch from c32a834 to 9874e64 Compare June 21, 2023 21:57
@soehms
Copy link
Copy Markdown
Member Author

soehms commented Jul 4, 2023 •
edited
Loading

I don't know why all GitHub bots except the one building the documentation have been cancelled? Anyway, the changes of the branch do only effect src/sage/knots/knotinfo.py and here all tests are fine.

@soehms soehms requested a review from tscrim July 4, 2023 16:25
tscrim
tscrim requested changes Jul 5, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Just a few very minor things.

tscrim
tscrim approved these changes Jul 6, 2023
View reviewed changes
Copy link
Copy Markdown
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. LGTM.

@soehms
Copy link
Copy Markdown
Member Author

soehms commented Jul 6, 2023

Thank you. LGTM.

Thank you, as well!

@soehms soehms mentioned this pull request Jul 10, 2023
Merged
4 tasks
@kwankyu
Copy link
Copy Markdown
Collaborator

kwankyu commented Jul 10, 2023

@vbraun: strangely this PR is still open.

@soehms
Copy link
Copy Markdown
Member Author

soehms commented Jul 11, 2023

@vbraun: strangely this PR is still open.

It seems that indeed commit 3291f4d (which just removes a blank-line) has not been merged. Probably this keeps it open.

@vbraun
Copy link
Copy Markdown
Member

vbraun commented Jul 19, 2023

Merge conflict

@soehms
Copy link
Copy Markdown
Member Author

soehms commented Jul 20, 2023

Merge conflict

@vbraun there is some mystery with this PR. It is already merged into 10.1.beta6 but just one commit (3291f4d) remains off (which just removes a blank line, see #35800 (comment) and #35172 (comment) at BTW).

Now, if I merge this branch with develop and try to push it back, git says Everything up-to-date. So what can I do?

Note my review comment: #35800 (comment): With the missing commit there has been something irregular from the beginning: It was not visible on the PR (at least for a couple of hours) after I've pushed it. Maybe this is related to the problem now!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 22, 2023

Documentation preview for this PR (built with commit ab0a42e; changes) is ready! 🎉

@soehms soehms closed this Jul 23, 2023
@soehms soehms reopened this Jul 23, 2023
@soehms soehms changed the title Adapt the KnotInfo interface to new Khovanov polynomial data Adapt the KnotInfo interface to new Khovanov polynomial data (merged) Jul 23, 2023
@soehms soehms closed this Jul 23, 2023
@soehms soehms deleted the khovanov_knotjob_adaption branch July 23, 2023 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tscrim tscrim tscrim approved these changes

Assignees

No one assigned

Labels

c: packages: optional t: enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@soehms @kwankyu @vbraun @tscrim