Doing some cleanup on Littelmann paths

[tscrim]

We do some cleanup on Littelmann paths by allowing greater input (including covering a surprising case when passing in a Cartan type and a weight for finite type), some PEP8 stuff, some doc formatting, commenting out assert statements, and other misc improvements. This is not necessarily meant to be comprehensive, but it moves things in the right direction. The most important thing is fixing the surprising input behavior.

📝 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]

@bsalisbury1

[bsalisbury1]

This is my first attempt at using the new GitHub setup with Sage (I know, I'm very slow), so I'm not sure how to officially make comments on a ticket. I'm having trouble building the documentation on my machine anyway (probably because I'm so far behind on builds -- even though 10.3.beta4 builds fine). Anyway, here are some comments:

• Are f-strings not being used in Sage by default yet? For example, I would replace line 256 with

f"The crystal of LS paths of type {self._cartan_type} and weight {self.weight}"

• Why are the assert lines, like line 353, being commented out rather than deleted outright?
• For consistency, line 517: "Return the dualized path of self."
• Delete "the" from the beginning of ValueError in line 719.

Otherwise, I think it looks good.

[tscrim]

This is my first attempt at using the new GitHub setup with Sage (I know, I'm very slow), so I'm not sure how to officially make comments on a ticket. I'm having trouble building the documentation on my machine anyway (probably because I'm so far behind on builds -- even though 10.3.beta4 builds fine).

For the docbuilds, this is now done automatically, which can be checked in the link right above your comment. So you don't have to do it yourself anymore.

* Are f-strings not being used in Sage by default yet?  For example, I would replace line 256 with

Changed; although I personally find f-strings to be a bit more annoying than the % and .format().

* Why are the `assert` lines, like line 353, being commented out rather than deleted outright?

In case we wanted to have proper checks at some point, but I removed them.

* For consistency, line 517: "Return the dualized path of `self`."

Done.

* Delete "the" from the beginning of `ValueError` in line 719.

I did not remove this as it would make the message use very weird English.

[bsalisbury1]

This is my first attempt at using the new GitHub setup with Sage (I know, I'm very slow), so I'm not sure how to officially make comments on a ticket. I'm having trouble building the documentation on my machine anyway (probably because I'm so far behind on builds -- even though 10.3.beta4 builds fine).

For the docbuilds, this is now done automatically, which can be checked in the link right above your comment. So you don't have to do it yourself anymore.

That's a nice feature!

• Delete "the" from the beginning of ValueError in line 719.

I did not remove this as it would make the message use very weird English.

I don't know that it's weird. In any case, the suggestion was based on the change you made on line 180.

Beyond that, the bot above is reporting Build & Test / build (pull_request) failure. It looks like there are some failed doctests. I can replicate the failures on my machine.

...
      File "sage/modules/with_basis/indexed_element.pyx", line 51, in sage.modules.with_basis.indexed_element.IndexedFreeModuleElement.__init__
        def __init__(self, M, x):
    TypeError: __init__() takes exactly 2 positional arguments (1 given)
**********************************************************************
1 item had failures:
   6 of  28 in sage.combinat.crystals.littelmann_path.CrystalOfProjectedLevelZeroLSPaths.one_dimensional_configuration_sum
    [321 tests, 6 failures, 4.60 s]
----------------------------------------------------------------------
sage -t --long --random-seed=82460175991284101813365755524236173996 src/sage/combinat/crystals/littelmann_path.py  # 6 doctests failed
----------------------------------------------------------------------

[tscrim]

• Delete "the" from the beginning of ValueError in line 719.

I did not remove this as it would make the message use very weird English.

I don't know that it's weird. In any case, the suggestion was based on the change you made on line 180.

I ended up deleting too much on line 180. I added the "the" back in there as it is weird without it there too.

Beyond that, the bot above is reporting Build & Test / build (pull_request) failure. It looks like there are some failed doctests. I can replicate the failures on my machine.

Fixed. It was only being tested with # long time tests, which I forgot to check before pushing.

[github-actions]

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

[bsalisbury1]

Everything now looks good to me. I still don't see a way to give this a "positive review"...

[tscrim]

@mkoeppe Please add Ben @bsalisbury1 to the "triage" team so he can approve the PR.

[mkoeppe]

When I looked, @bsalisbury1 was already on Triage, so someone seems to have been faster!

[tscrim]

Thank you, likely to @fchapoton for doing that. (Actually, I just realized I probably have sufficient powers now to do that myself...)

@bsalisbury1 If everything is still good with you, then please approve the PR (under the "files changed" and "review changes" button).

[bsalisbury1]

Looks good.