Jamie/save multiple charts

[charlesknightsbridge]

I'm confused. This shows all the commits leading up to the current Master, I think. I merged all the changes to Master into this to make sure there was nothing missed out.

@torstenchr should I make a new branch and bring all of these changes in? I see that the changed files only include the newly changed ones. Please let me know what you want me to do here.

Pareto (in master) can create multiple chart pairs with the !by command. Now we can actually save these charts properly. At the moment, master cannot save charts from the command line (error) and saving from the chart dialog gives weird file names. I think I've used a reasonable way to manage this, with minimal changes to the save graph units.

And I just noticed that pareto has been added to command.html

[torstenchr]

I think the problem was that you merged the master into the branch instead or rebasing. They are two different methods and rebasing is often the preferred method when the "master" branch is ahead of the current feature branch. Rebasing a feature branch on the master branch can be done like this:

git checkout <feature-brach>

git pull #make sure the feature branch is up-to-date

git fetch --all #update meta-data of all branches, including master

git rebase origin/master

[charlesknightsbridge]

I looked at rebase; I’m sure I’ve done this before, but the conflict resolution left my head spinning. Reading a bit on Git help suggested to me that unless I really knew what I was doing, merge is safer. I can see in the pull request, the actual changes I’ve made are all confined to the last commit shown on that page. I could make a new branch from master and reproduce the changes. It will be a lot cleaner (without all the history of master), but…

I’m a bit reluctant to do anything, but whatever you think best, I’ll do (except when I looked at rebase, the conflicts seemed a challenge to get right).

I’m quite amazed that I even got this to work; I’d just about given up with the approach I was taking, but it all seems to make sense now.

On May 6, 2024, at 3:38 PM, Torsten Bonde Christiansen @.***> wrote:

I think the problem was that you merged the master into the branch instead or rebasing. They are two different methods and rebasing is often the preferred method when the "master" branch is ahead of the current feature branch. Rebasing a feature branch on the master branch can be done like this:

`git checkout

git pull #make sure the feature branch is up-to-date

git fetch --all #update meta-data of all branches, including master

git rebase origin/master`

— Reply to this email directly, view it on GitHub https://github.com/EpiData-dk/analysis/pull/73#issuecomment-2096770196, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQYAF7CNUBN2T4C24AQI3ZA7LZTAVCNFSM6AAAAABHJTJ2NSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJWG43TAMJZGY. You are receiving this because you authored the thread.

[charlesknightsbridge]

I tried rebase and it ended up the same, with all of the commits from master included in the branch. EXCEPT I am not completely certain that other changes didn't creep in as I resolved all the conflicts.

There is still more, but I'll do those separately:

• include BY_VARIABLE in pareto variables dialog
• information in the help file about saving multiple charts
• in the chart save dialog, we need to check the existence of the actual file names when there are multiple charts, not just the base name
• we need to agree that the multiple charts are saved the way I've specified (name-stratumValue.ext)

[torstenchr]

I'm sorry Jamie, but it is really hard to see what you are working on here - are you trying to implement multiple features?

You are adding Stratum Values to a single chart, and adding thing into the pareto graph. Even the executor is getting things reworked...

The heading is "save multple charts" but that seems to be only part of this PR.

Is there some way you can separate the work into multiple smaller PR's that are more clear?

[charlesknightsbridge]

Everything before the last commit are commits already merged into Master. They show up this way because I had merged master into this branch, rather than rebasing. When I tried rebasing, I still saw all of the old commits and I could not be sure that I resolved the multiple conflicts the right way.

Previous commits made back end changes to support the !by option in charts, which was implemented in pareto. !by implies that there will be one chart for each stratum. This is different from the current display of multiple strata in a single chart (histogram, epicurve, bar, survival).

The !by requirement arose because of pareto, where you cannot plot multiple strata on the same chart because the x-axis changes. To put them in the same graph frame would be possible, but becomes impractical if you have more than a couple of strata.

We already have multiple charts as tabs within the graph form. (part of the big merge for pareto)

My approach, when I got back to this, was:

1. define the back end support in graphexecutor and graphform (merged)
2. implement in pareto (save will fail) (merged)
3. implement graph save (this one)

I thought I had documented the changes. The trick was to get save files named in a simple, but logical, way (using the stratum value). I had originally used the stratum value label, but that was a bad idea. The stratum value is simpler. That's what led to changes in the back end for graphs.

[charlesknightsbridge]

splitting this up with branches based on today's master.

[charlesknightsbridge]

Hi Torsten, I’m sending this outside of github… I’ve just made a sequential series of pull requests implementing saving multiple charts from a single graph command. This only works for pareto. The pull request build on each other — I based each new branch on the one in the previous pull request. This breaks up that big PR and if you see something amiss as I went along, it will be easier to fix. Cheers, Jamie

On May 15, 2024, at 3:21 PM, Torsten Bonde Christiansen @.***> wrote:

I'm sorry Jamie, but it is really hard to see what you are working on here - are you trying to implement multiple features?

You are adding Stratum Values to a single chart, and adding thing into the pareto graph. Even the executor is getting things reworked...

The heading is "save multple charts" but that seems to be only part of this PR.

Is there some way you can separate the work into multiple smaller PR's that are more clear?

— Reply to this email directly, view it on GitHub https://github.com/EpiData-dk/analysis/pull/73#issuecomment-2113298578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQYAEHM7Y6OP3BLPQYAHLZCOYSPAVCNFSM6AAAAABHJTJ2NSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJTGI4TQNJXHA. You are receiving this because you authored the thread.