Fix revamp of seats positioning

[Gouvernathor]

Fix #113. The gitch in the number of seats is solved, the output is now similar to what my other code gives me, so if you don't like the result at least there's no bug distorting it. I tried all numbers of seats until 4 rows, and randomly afterwards, I saw no difference but for the 1-seat newarch (but who cares ?).

Also it's now easier to test the main function, you can pass it the kind of dict you stored in the json file (just replace false with False).

[slashme]

Thanks, I'll take a look when I get home this afternoon!

[slashme]

@Gouvernathor - I don't know how to pass a parameter to the main function of this script when I'm running it. I've only ever run the script from the commandline, passing it arguments which get read using sys.argv, or by using cgi.fieldstorage when it's running as a web service. Please help :-D

[Gouvernathor]

In the very last part of the script, in the if __main__, call the main() function with a dict instead of nothing. Example :

for k in range(300):
    inputlist = {
          "denser_rows": False,
          "parties": [
            {
              "name": "Party 1",
              "nb_seats": k,
              "color": "#95C241",
              "border_size": 0,
              "border_color": "#000000"
            }
          ]
        }
    main(inputlist=inputlist)

Put this in the if at the end of the file, maybe check the indentation, should generate the first 300 diagrams (actually 299 since range starts iterating at 0 and no seats is skipped).

[Gouvernathor]

You could also try and read the argv, but I find it a lot simpler to just edit and run the py file, including any version of the tests you want to run.

[Gouvernathor]

Hi ! Just checking in if you forgot about this. But no pressure ! ❤

[slashme]

Hello!

I've still got your emails in my inbox, reminding me to get around to it, but I just haven't :-D

Absolutely on my to-do list as soon as my wild and crazy life settles down...

David Richfield +49 176 7266 3368

On Fri, 19 Nov 2021 at 14:31, Gouvernathor @.***> wrote:

Hi ! Just checking in if you forgot about this. But no pressure ! ❤

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/slashme/parliamentdiagram/pull/115#issuecomment-974075204, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADFGK2EFTJRZNAK32UBOSLUMZGRVANCNFSM5EKIB5AQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[slashme]

Hi @Gouvernathor ,

I've finally gotten back to real life, and I've started looking at your code again. I quickly tested it, and it seems that you removed the dense rows functionality. Was that intentional?

I'm going to start looking at the latest code by @Rade-Mathis now, will integrate yours once I'm done.

Thanks again for your help!!

[Gouvernathor]

It's partly intentional. Dense rows were added, if I'm not mistaken, because the formula for circles' width wasn't adapted to larger parliaments, so it added a second formula dedicated for that other case. My version's formula makes it constant relative to the available room, so it short-circuits the need for the "dense rows" option.

[slashme]

OK, I just checked, and for example, a parliament with 5 seats really doesn't look good without the denser-rows option, either in my original code, or with this update, so I'd like to keep that option that gives a functionality something like that for smaller parliaments.

[Gouvernathor]

Point taken. Since my formula is supposed to be optimized, I'll try to understand how both ways currently work, and whether my math is wrong, or there's something mathematically inexact about the dense_rows option. If it is so, and I suppose it is, I'll see how I can translate it in my code.

[Rade-Mathis]

The idea of having "denser rows" as an option, is because going for one or another behavior shouldn't be a developer's decision, but a user's one.

In this kind of situation, the denser row (2nd image) is clearly better :

But in this situation It's harder to say that one is better than another. What's worse? The sparseness of the 1st one? Or the thinness of the 2nd? By experience, different users won't agree on which one is the best. So we settled for an adaptable situation: Let any user do how they please.

[Gouvernathor]

I reimplemented a denser_rows option ! It works (math-wise and in purpose) almost exactly like the original denser_rows option, a.k.a the old optimize_rows function, except it doesn't use magic numbers, and I think (and hope) it's more clear to understand and to maintain in the future. To make it clear : it tries to stuff as many seats as possible on the outer rows in, then remembers how many rows it needs at the minimum to store all seats, and then it evens the seats out between the required outer rows.

You can test it using the code I wrote here but with denser_rows being True.

[Gouvernathor]

Bonus : a display of what my math revamp yields (denser_rows unrelated, I could have drawn that figure sooner). The constraints are written at the bottom. This image can be kept for posterity, if someone needs to explore my formulae. By the way, radius (the radius of one seat, before it's diminished by 20% to make them not touch one another) is FH on the picture, so that's where the formula comes from and why to be accurate it needs to be a bit more complex than just some constant divided by nb_rows.

This math is not equivalent to what the no-revamp version yields : for some reason in the current version the innermost seats visibly go way more towards the center (see this one for example).