Upgrade database_knotinfo to version 2024.2.1

[soehms]

The upgrade is necessary by two reasons:

1. Since version 2023.10.1 knots with 13 crossings have been added to the database. The new data could not be used entirely, firstly because of missing adaptions on the Sage side and secondly because the HOMFLY-PT data has been incompatible on the KnotInfo side. The latter has been fixed in version 2024.1.1.
2. The old table columns khovanov_polynomial and khovanov_polynomial_torsion have been removed from the database for knots. Since these have been kept in Sage 10.1 for optional usage this breaks compatiblity with 2024.1.1 completely.

This PR restores compatibility with the current content of the database in both cases.

Furthermore it adds KnotInfo and KnotInfoSeries to the global namespace, even in the case where only the demo part of the database is present. I made the former restriction because I unnecessarily feared this could affect the startup-time of Sage.

Finally I changed a couple of inline doctest tags # optional database_knotinfo to according block-scoped tags. Note, that this causes 2 doctest-warnings:

**********************************************************************
File "src/sage/knots/link.py", line 4070, in sage.knots.link.Link.get_knotinfo
Warning: Variable 'KnotInfo' referenced here was set only in doctest marked '# optional - database_knotinfo'
    K = KnotInfo.K10_25
**********************************************************************
File "src/sage/knots/link.py", line 4104, in sage.knots.link.Link.get_knotinfo
Warning: Variable 'KnotInfo' referenced here was set only in doctest marked '# optional - database_knotinfo'
    L.get_knotinfo(mirror_version=False) == KnotInfo.K0_1

But I think these warnings are baseless, since KnotInfo is in the global namespace. I guess this is a bug in doctest functionality.

📝 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

[tscrim]

I can review this when ready.

[soehms]

I can review this when ready.

Very kind! In the meantime I've made some cross-checks with the new data. HOMFLY, Jones and Conway polynomials match the results calculated by Sage. With respect to Khovanov homology I compared with the computations of Khoca (because of performance reasons) using the PyPI version I created 2021 (note that this is not stable software, see LLewark/khoca#3). This cross-check found four differences for reduced homology in characteristic 2:

sage: cmp_pd_13_poly_char2_red
[<KnotInfo.K13n_3663: '13n_3663'>,
 <KnotInfo.K13n_4587: '13n_4587'>,
 <KnotInfo.K13n_4639: '13n_4639'>,
 <KnotInfo.K13n_5016: '13n_5016'>]

I will report this to Chuck Livingston. But maybe you can check independently that everthing is o.K. on the Sage side.

[soehms]

I pushed the version of database_knotinfo to the current 2024.2.1 and made some further optional doctests in link.py block-scoped.

[tscrim]

I am worried that you are doing something slightly strange with the control logic given the error messages. Empty strings versus something more clear like None for invalid data seems better. If you think empty strings are best, then I think some more documentation is needed.

Also, I think we should deprecate the KhoHo attribute. Although could there be a use for having it to select which database to use?

[soehms]

I am worried that you are doing something slightly strange with the control logic given the error messages. Empty strings versus something more clear like None for invalid data seems better. If you think empty strings are best, then I think some more documentation is needed.

I'm not sure I understand your concern! The empty strings are used in the database when no value is specified for a knot or link. Such cases have increased in the new data for knots with 13 crossings, as not all properties can be easily calculated (as with the braid_index). Raising a ValueError for these could be confusing for the user of the interface since he didn't pass a value to the method. Or do you want to change empty strings to None already in the Python wrapper for the Excel sheets?

Also, I think we should deprecate the KhoHo attribute. Although could there be a use for having it to select which database to use?

The "KhoHo" attribute results in an IndexError if the database is installed with version "2024.1.1" or newer. I could catch the IndexError, ignore the attribute, and display a deprecation warning indicating a database version downgrade. If a compatible version is installed, I would show a different deprecation warning and continue using the attribute. Are you suggesting that?

[tscrim]

I am worried that you are doing something slightly strange with the control logic given the error messages. Empty strings versus something more clear like None for invalid data seems better. If you think empty strings are best, then I think some more documentation is needed.

I'm not sure I understand your concern! The empty strings are used in the database when no value is specified for a knot or link. Such cases have increased in the new data for knots with 13 crossings, as not all properties can be easily calculated (as with the braid_index). Raising a ValueError for these could be confusing for the user of the interface since he didn't pass a value to the method. Or do you want to change empty strings to None already in the Python wrapper for the Excel sheets?

Okay, I understand, I didn't realize empty strings were used for unknown values in the database. It's a little strange that this isn't converted to, e.g., a None earlier, but now I understand what your code is checking for. It might be good to put a code comment about this.

Also, I think we should deprecate the KhoHo attribute. Although could there be a use for having it to select which database to use?

The "KhoHo" attribute results in an IndexError if the database is installed with version "2024.1.1" or newer. I could catch the IndexError, ignore the attribute, and display a deprecation warning indicating a database version downgrade. If a compatible version is installed, I would show a different deprecation warning and continue using the attribute. Are you suggesting that?

I would give a deprecation message (and then ignore the parameter) if the user passes it (for example; their code explicitly was passing the argument before the Sage update). That way we break less code and follow standard Sage practices.

I also wasn't sure if the parameter was still useful or not in the upgraded version (as you said, it is not).

[soehms]

.... It might be good to put a code comment about this.
.....
I would give a deprecation message (and then ignore the parameter) if the user passes it (for example; their code explicitly was passing the argument before the Sage update). That way we break less code and follow standard Sage practices.

Both tasks I've done in the current commit.

[tscrim]

Thank you. One last little thing, then you can set a positive review, change the error messages to start with a lowercase letter (as we follow Python's convention).

[soehms]

Thank you. One last little thing, then you can set a positive review, change the error messages to start with a lowercase letter (as we follow Python's convention).

Thanks, as well!

[github-actions]

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