Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different formatting of arrays and matrices #29

Open
tpwo opened this issue Aug 16, 2021 · 4 comments
Open

Different formatting of arrays and matrices #29

tpwo opened this issue Aug 16, 2021 · 4 comments

Comments

@tpwo
Copy link
Contributor

tpwo commented Aug 16, 2021

Hi, I noticed that arrays ({}) and matrices ([]) are formatted quite differently.

Arrays are always put into single line (if inside length limit) and matrices are always split per each row.

Let's demonstrate this on a simple example:

Before formatting:

within Somewhere;
model ArraysFormatting
  "An example to demonstrate formatting of arrays and matrices"

  parameter Real arrayA[3,3] = {{1.0, 2.0, 3.0}, {4.0, 5.0, 6.0}, {7.0, 8.0, 9.0}};

  parameter Real arrayB[3,3] = {
    {1.0, 2.0, 3.0},
    {4.0, 5.0, 6.0},
    {7.0, 8.0, 9.0}};

  parameter Real matrixA[3,3] = [1.0, 2.0, 3.0; 5.0, 6.0, 7.0; 7.0, 8.0, 9.0];

  parameter Real matrixB[3,3] = [
    1.0, 2.0, 3.0;
    4.0, 5.0, 6.0;
    7.0, 8.0, 9.0];

end ArraysFormatting;

After formatting:

within Somewhere;
model ArraysFormatting
  "An example to demonstrate formatting of arrays and matrices"
  parameter Real arrayA[3,3]={{1.0,2.0,3.0},{4.0,5.0,6.0},{7.0,8.0,9.0}};
  parameter Real arrayB[3,3]={{1.0,2.0,3.0},{4.0,5.0,6.0},{7.0,8.0,9.0}};
  parameter Real matrixA[3,3]=[
    1.0,2.0,3.0;
    5.0,6.0,7.0;
    7.0,8.0,9.0];
  parameter Real matrixB[3,3]=[
    1.0,2.0,3.0;
    4.0,5.0,6.0;
    7.0,8.0,9.0];
end ArraysFormatting;

Is there any reason for that? I guess that it might be connected with annotations, as arrays are used there and one-line syntax works probably works better with that. But outside of annotations, I think that the clearer format is something similar to the current formatting of matrices.

Also, arrays can be multidimensional, so output like that probably makes the most sense:

  parameter Real array3D[2,2,2] = {
    {
      {1.0, 2.0},
      {1.0, 2.0}},
    {
      {1.0, 2.0},
      {1.0, 2.0}}};

What do you think? Is it a big challenge to change? I think that the current code is able to detect if parser is inside annotation, isn't it?

@macintoshpie
Copy link
Contributor

This is a good point, we should format multidimensional arrays as we do matrices. We do have a way of tracking if we are inside of an annotation, so you're right, maybe we could use that to determine if we should do this type of foratting.

One concern I have is determining when to linebreak in the array. e.g. if we just did curly braces we might end up with something like this:

  parameter Real array3D[2,2,2] = {
    {
      {
        1.0, 2.0},
      {
        1.0, 2.0}},
    {
      {
       1.0, 2.0},
      {
        1.0, 2.0}}};

Another question, how do we handle arrays with more than 3 dimensions (e.g. a 4d array)?

I don't think I'll have much time to look deeply into this right now, but I think this would definitely be an improvement.

@tpwo
Copy link
Contributor Author

tpwo commented Aug 20, 2021

I thought a bit on it and I think that I have a consistent solution.

Here is an example for 2D and 3D:

parameter Integer array2D[2,2] = {
  {1,2},
  {3,4}
};

parameter Integer array3D[2,2,2] = {
  {
    {1,2},
    {3,4}
  },
  {
    {5,6},
    {7,8}
  }
};

And now, the explanation of the rules that should make the above possible. With * I marked either { or } I'm looking on in a given line if this is not obvious. I'm looking at given { / } and the next symbol.

Opening brace {

  • If it's the first opening { -> break line and add a single indentation level

    parameter Integer array3D[2,2,2] = {  // here
      {
    <...>

    This is fine for big arrays, but would be strange for small ones. Maybe we can break line after the first { only if the array is 2D or more? But again, even a 1D array can be very long and in this case something like this will be more legible than breaking after X characters (with -line-length option set):

    parameter Integer array1D[100] = {
      1,
      2,
      <...>
      99,
      100
    };

    So a better solution would be to break only if array is big enough based on its total length (this can be also addressed in case of matrices, currently they always break after the first [).

    I'm not sure about the way to achieve it. If counting the number of characters between the outer { and } is feasible, then it could be a way to go.

  • If the next symbol is another { -> break line and increase indentation level

    parameter Integer array3D[2,2,2] = {
      {           // here
        {1,2},
    <...>
  • Otherwise -> no action, keep current line

    parameter Integer array3D[2,2,2] = {
      {
        *{*1,2},  // here
    <...>

    Note that this would not scale well if the last dimension is very large:

    parameter Integer array3D[2,2,200] = {
      {
        *{*1,2,3,<...>,198,199,200},  // here
    <...>

    Probably similar treat that I described above regarding very long 1D arrays can be used:

    parameter Integer array3D[2,2,200] = {
      {
        {          // here
          1,
          2,
          3,
          <...>
          198,
          199,
          200
        },
        {
    <...>

    But it could grow really long, so I'm not that convinced about this solution.

Closing brace }

  • If the next symbol is comma (ie ,) -> break line after , and keep current level of indentation

    parameter Integer array3D[2,2,2] = {
      {
        {1,2*}*,  // here
        {3,4}
    <...>

    or

    parameter Integer array3D[2,2,2] = {
      {
        {1,2},
        {3,4}
      },         // here
      {
        {5,6},
    <...>
  • If the next symbol is another } -> break line and decrease indentation

    parameter Integer array3D[2,2,2] = {
      {
        {1,2},
        {3,4*}*  // here
      },
    <...>
  • If the next symbol is ; -> it's the end of the array. With the above rules applied, }; should be indented the same as the first line of array declaration, ie:

    parameter Integer array2D[2,2] = {  // the indentation of this line
      {1, 2},
      {3, 4}
    };                                  // and this line is the same

Beyond 3D

It seems that this should scale well if we keep on adding new dimensions. Here is an example with 4D:

parameter Integer array4D[2,2,2,2] = {
  {
    {
      {1,2},
      {3,4}
    },
    {
      {5,6},
      {7,8}
    }
  },
  {
    {
      {9,10},
      {11,12}
    },
    {
      {13,14},
      {15,16}
    }
  }
};

I need to admit that this proposal is heavily inspired by the output of black, code formatter that I use a lot. It formats Python lists of lists of lists in a very similar way (but Python allows for trailing commas, so their approach is probably a bit more complicated :D)

I hope that it was clear enough. What do you think?

@macintoshpie
Copy link
Contributor

Hey @trivvz thanks for the suggestions! What are yall's thoughts on this @mwetter @nllong ?

@mwetter
Copy link

mwetter commented Sep 17, 2021

I would much prefer a more compact layout, such as

parameter Integer array4D[2,2,2,2] = {
  {{{1,2}, {3,4}},
    {{5,6}, {7,8}}},

When looking at source code it is in my view important to see a large part of code.

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

No branches or pull requests

3 participants