Multiple refactorings reformatted

[alyst]

This is a rebase of a #181 with JuliaFormatter.jl applied to each commit with this command:

git rebase `
  --strategy-option=theirs `
  --exec "julia -e 'using Pkg; Pkg.activate(\`"$env:JULIA_PROJECT\`"); using JuliaFormatter; format(\`".\`")' && git add . && git commit --amend --no-edit --allow-empty" `
  main

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 59.36508% with 384 lines in your changes missing coverage. Please review.

Project coverage is 65.34%. Comparing base (8b0f880) to head (486984b). Report is 16 commits behind head on devel.

Files with missing lines	Patch %	Lines
src/frontend/predict.jl	0.00%	57 Missing :warning:
src/frontend/specification/ParameterTable.jl	0.00%	54 Missing :warning:
src/frontend/specification/Sem.jl	60.58%	54 Missing :warning:
ext/SEMBlackBoxOptimExt/DiffEvoFactory.jl	0.00%	51 Missing :warning:
ext/SEMBlackBoxOptimExt/BlackBoxOptim.jl	0.00%	22 Missing :warning:
...t/SEMBlackBoxOptimExt/SemOptimizerBlackBoxOptim.jl	0.00%	21 Missing :warning:
src/observed/EM.jl	82.72%	19 Missing :warning:
src/additional_functions/sparse_utils.jl	53.84%	18 Missing :warning:
ext/SEMBlackBoxOptimExt/AdamMutation.jl	0.00%	17 Missing :warning:
src/frontend/specification/RAMMatrices.jl	23.52%	13 Missing :warning:
... and 18 more

Additional details and impacted files

@@            Coverage Diff             @@
##            devel     #193      +/-   ##
==========================================
- Coverage   72.94%   65.34%   -7.61%     
==========================================
  Files          50       60      +10     
  Lines        2218     2542     +324     
==========================================
+ Hits         1618     1661      +43     
- Misses        600      881     +281     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[alyst]

Note that for format suggestions to work, you need to add write permissions to GITHUB_TOKEN, that's the output from the action:

reviewdog: This GitHub Token doesn't have write permission of Review API [1],
so reviewdog will report results via logging command [2] and create annotations similar to
github-pr-check reporter as a fallback.
[1]: https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request_target
[2]: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions

[Maximilian-Stefan-Ernst]

Cool! Now we have to decide how to merge this... I see a few possibilities:

1. Make a huge review of this PR and merge everything at once.
2. I try to break it up into smaller PR's and merge piece by piece, beginning with the first commit, and cherry-picking commits related to different "topics" onto different branches.
3. You may have some ideas which features are orthogonal, and we could merge these in separate PR's first, and then maybe merge the stuff that can not be separated together.

I'm not sure what the best solution is, what do you think?

[Maximilian-Stefan-Ernst]

Regarding the format suggestions: I'm not sure why it does not work, it has to do with your PR coming from a fork, because here it works: #194. I tried setting pull_request_target, which should give GITHUB_TOKEN write permission, but it does not seem to fix the problem. See also this: JuliaDiff/ChainRulesCore.jl#489

[alyst]

I tried setting pull_request_target

You mean adjusting the triggers in FormatCheck.yml to pull_request_target?

Now we have to decide how to merge this... I see a few possibilities

My first commits were less invasive and should be easier to review. I think up to SemSpecifications: vars API. Parts of these changes (e.g. RAMMatrices tweaks) were later superseded by more substantial changes, but I guess that's fine to have them in the commit history. I'll try to cherry-pick these initial changes into separate PRs that you can review more easily.

Then, once it's reviewed and landed in main or devel, it would be easier to rebase this PR and extract the next smaller PR. And so on.

[Maximilian-Stefan-Ernst]

You mean adjusting the triggers in FormatCheck.yml to pull_request_target?

Yes, it's here (I changed the base of your PR to devel).

I'll try to cherry-pick these initial changes into separate PRs that you can review more easily.

Perfect, thanks!

[alyst]

The formatting check does seem to work now, the previous failure was because it was still triggered by pull_request.

[Maximilian-Stefan-Ernst]

Ah interesting, I re-ran it manually and assumed it would then use the new configuration ^^

[Maximilian-Stefan-Ernst]

@alyst I'm thinking about making a new release soon, but I don't necessarily want to have 2 breaking releases - do you think we can integrate all changes that break user code soon, and add new features after that for minor releases, or is this unrealistic?

[alyst]

@Maximilian-Stefan-Ernst Sorry for delaying the process. I am currently very busy at work. I have already rebased this branch to the new devel, but have not prepared the next smaller PR yet. That would be the refactoring of function/gradient/hessian evaluation. This change is less user-visible, but the next one would be the introduction of ParamsArray to simplify the RAMMatrices code, which may affect the users. And the next one yet -- potentially, the unification of the optimizer interface, which changes how user runs SEM inference (not significantly, but the old code would require changes). So I'm afraid, given my current workload, it would be hard to merge everything within a few days/weeks. But I think it is fine to make new releases if necessary, given the package is still at 0.x stage.

[Maximilian-Stefan-Ernst]

Alright, no worries, just wanted to know what makes sense. In this case, I would just make releases along the way as I see fit!