Skip to content

Revert "Hopefully fix the broken tutorials"#12

Open
lgoettgens wants to merge 6 commits intomasterfrom
revert-9-ak96/fix-new-json
Open

Revert "Hopefully fix the broken tutorials"#12
lgoettgens wants to merge 6 commits intomasterfrom
revert-9-ak96/fix-new-json

Conversation

@lgoettgens
Copy link
Copy Markdown
Member

Reverts #9

To be merged once Nemocas/AbstractAlgebra.jl#2209 is merged and released via Nemocas/AbstractAlgebra.jl#2210.

@lgoettgens lgoettgens requested a review from aaruni96 November 7, 2025 13:42
@aaruni96
Copy link
Copy Markdown
Member

aaruni96 commented Nov 7, 2025

We can't just revert that change, unfortunately. The new IJulia also subtly changes how they write their json (expanded over multiple lines in the old release v/s all in one line in the new release).

It will also require the change from a974452 , and maybe some debugging in case that doesn't directly work. (The key part is adding jq into the pipe, so that the json we give to sed is expanded over multiple lines again. An alternative would be to change the sed command to be more surgical than removing the entire line.)

@aaruni96
Copy link
Copy Markdown
Member

aaruni96 commented Nov 7, 2025

It is possible the step to use the default project is not needed at all, since JuliaLang/IJulia.jl#928 was closed, and should be removed. Experimentation is required.

@aaruni96
Copy link
Copy Markdown
Member

aaruni96 commented Nov 7, 2025

Plus, in my opinion, it is not desirable to have the github action run on every pull request.

Copy link
Copy Markdown
Member

@aaruni96 aaruni96 left a comment

Choose a reason for hiding this comment

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

See above comments (sorry, I am new to the review featureset)

@lgoettgens
Copy link
Copy Markdown
Member Author

Plus, in my opinion, it is not desirable to have the github action run on every pull request.

I don't really care about the trigger, that was just part of the PR I reverted. This is just about dropping the version pin again.

@JamesWrigley
Copy link
Copy Markdown

We can't just revert that change, unfortunately. The new IJulia also subtly changes how they write their json (expanded over multiple lines in the old release v/s all in one line in the new release).

Sorry about that, the last release seems to have been even more breaking than I thought 🙃 If I understand correctly you need to remove the --project=@ argument? If so then I would recommend just installing the kernel yourself with IJulia.installkernel("Julia"). That will overwrite the default one, and if you like you can also set the IJULIA_NODEFAULTKERNEL environment variable to prevent a default kernel being installed at all.

Docs for creating kernels are here: https://julialang.github.io/IJulia.jl/stable/manual/installation/#Installing-and-customizing-kernels

Comment thread .github/workflows/CI.yml Outdated
@lgoettgens
Copy link
Copy Markdown
Member Author

With #14, the other changes by @aaruni96 should be obsolete. I've started a CI run to verify in https://github.com/oscar-system/TutorialTesterforOscar/actions/runs/19703572022.

Copy link
Copy Markdown
Member

@aaruni96 aaruni96 left a comment

Choose a reason for hiding this comment

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

I think we still need these changes to install a new kernel which does not use the named project @, and just uses the default project?

(See #12 (comment) )

Comment thread .github/workflows/CI.yml
Comment thread .github/workflows/CI.yml
Co-authored-by: Aaruni Kaushik <aaruni96@users.noreply.github.com>
@aaruni96
Copy link
Copy Markdown
Member

aaruni96 commented Mar 11, 2026

One can merge this once all tests pass on the action

https://github.com/oscar-system/TutorialTesterforOscar/actions/runs/22950621386

Edit: This probably needs a new release of OSCAR (once Nemocas/AbstractAlgebra.jl#2361 trickles through the release process and into OSCAR),

@lgoettgens
Copy link
Copy Markdown
Member Author

Edit: This probably needs a new release of OSCAR (once Nemocas/AbstractAlgebra.jl#2361 trickles through the release process and into OSCAR),

Nemocas/AbstractAlgebra.jl#2361 is available in AA v0.48.5, which is compatible with OSCAR v1.7.0.

@fingolfin
Copy link
Copy Markdown
Member

What is the status of this? @aaruni96 ? @lgoettgens ?

@lgoettgens lgoettgens mentioned this pull request Apr 17, 2026
@lgoettgens
Copy link
Copy Markdown
Member Author

This now runs into Nemocas/AbstractAlgebra.jl#2208, as the tests are performed with Oscar 1.5.1 (see #15), cf. https://github.com/oscar-system/TutorialTesterforOscar/actions/runs/24565614155/job/71824932191#step:9:40.

Once CI works with the latest Oscar release, I can try to push this over the finish line

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.

4 participants