factoring out common layout functionality #555
Conversation
|
|
||
|
|
||
| def app_layout_1(navbar, description, map, local): | ||
| """ |
There was a problem hiding this comment.
Just a comment—when I saw the '_1', I initially thought there were other types of layouts, like a '2', etc. Later I understood it was more to differentiate it from the main app layout definition. If we’re not going to have others, we could maybe give it a more descriptive name like "head, etc". In any case, it's not crucial—just something to keep in mind in case we need to create other versions of app_layout* later on.
There was a problem hiding this comment.
The _1 is actually in the expectation of _2 and others. But we've used only that layout so far. I thought of giving it a somewhat explicit name rather than a version number but I didn't find what that name could be so I opted for numbers.
I didn't give version numbers to the other entities that are put in a layout because: i) I am thinking that they can be changed with optional parameters, or if not would need entirely new entity; ii) Hopefully they are not going to be tight to one single layout and can be used in several different ones.
|
Why are you working on this now? Should I? I agree with the general principle of factoring out common stuff, but to comment on the particular way in which it was factored I would have to spend more time on it. |
|
What else to do? PepsiCo will likely need additional Python MRs. Now with both RSF awarded, more likely we'll convert forecast MRs (PepsiCo and Climate Group ones) to Python. If you have visibility on more useful/urgent stuff to do now, please share. |
The idea is that layout.py will contain only the specifics of a given app, using building blocks from layout_utilities.py that can be added to, with optional arguments in current functions, or new functions.
Will avoid copy-and-pasting all this stuff from Maproom to Maproom. Probably better engineering if we hand this over to someone.
Right now we have only one PepsiCo Maproom, but I am not sure the new deliverables will all fit in the existing one and it might call for one or more new ones; and we might want to port the Forecasts ones to Python if we are to shutdown classic MRs.
Could be applied to enacts.