Refactored `src/` and `tests/` files to be compliant to `rustfmt` #55

Merged
meliurwen merged 4 commits from 54-refactor-make-the-code-compliant-to-rustfmt into dev 2 years ago
meliurwen commented 2 years ago (Migrated from github.com)
Owner

Description

See the issue related, what is done in the commits and in the changelog below.

Merge into Dev Checklist

  • Tests for the new code
  • Acceptance Testing

Changelog

  • Refactored src/ files to be compliant to rustfmt
  • Refactored tests/ files to be compliant to rustfmt
  • Disabled tests/ folder for rustfmt

(Optional) Extra info

None

<!-- # Pull/Merge Request into dev --> ## Description See the issue related, what is done in the commits and in the changelog below. ## Issues related * #54 ## Merge into Dev Checklist * [x] Tests for the new code * [x] Acceptance Testing ## Changelog * Refactored `src/` files to be compliant to `rustfmt` * Refactored `tests/` files to be compliant to `rustfmt` * Disabled `tests/` folder for `rustfmt` ## (Optional) Extra info None
AlessandroBregoli (Migrated from github.com) requested changes 2 years ago
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

In my opinion, matrices was easier to read before the refactor.

In my opinion, matrices was easier to read before the refactor.
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
AlessandroBregoli (Migrated from github.com) commented 2 years ago
Owner

Same

Same
meliurwen commented 2 years ago (Migrated from github.com)
Owner

On the official rustfmt repo I've found that there's an ongoing issue open about the matrices topic.

At the moment probably an acceptable workaround could be to set rustfmt to skip all the files in tests/ folder and revert all the matrices.

@AlessandroBregoli

On the official `rustfmt` repo I've found that there's an [ongoing issue](https://github.com/rust-lang/rustfmt/issues/3312) open about the matrices topic. At the moment probably an acceptable workaround could be to set `rustfmt` to skip all the files in `tests/` folder and revert all the matrices. @AlessandroBregoli
meliurwen commented 2 years ago (Migrated from github.com)
Owner

I've spotted 3 different kinds of notations used for rank-3 tensors; I would suggest to use the same notation for clarity and consistency.

Here the 3 notations spotted, personally I find the notation number 2 the more clean:

[
    [[ 0,  2,  3],
     [ 4,  0,  6],
     [ 7,  8,  0]],
    [[0, 12,  90],
     [ 3, 0,  40],
     [ 6, 40,  0]],
    [[ 0,  2,  3],
     [ 4,  0,  6],
     [ 44, 66, 0]]
]
[
    [
        [ 0,  2,  3],
        [ 4,  0,  6],
        [ 7,  8,  0]
    ],
    [
        [0, 12,  90],
        [ 3, 0,  40],
        [ 6, 40,  0]
    ],
    [
        [ 0,  2,  3],
        [ 4,  0,  6],
        [ 44, 66, 0]
    ],
]
[
    [[-1.0, 0.5, 0.5], [3.0, -4.0, 1.0], [0.9, 0.1, -1.0]],
    [[-6.0, 2.0, 4.0], [1.5, -2.0, 0.5], [3.0, 1.0, -4.0]],
    [[-1.0, 0.1, 0.9], [2.0, -2.5, 0.5], [0.9, 0.1, -1.0]],
]

If you agree @AlessandroBregoli I would also proceed to conform all the rank-3 tensors in tests/ to the notation number 2 listed above.

I've spotted 3 different kinds of notations used for rank-3 tensors; I would suggest to use the same notation for clarity and consistency. Here the 3 notations spotted, personally I find the notation **number 2** the more clean: 1) ``` [ [[ 0, 2, 3], [ 4, 0, 6], [ 7, 8, 0]], [[0, 12, 90], [ 3, 0, 40], [ 6, 40, 0]], [[ 0, 2, 3], [ 4, 0, 6], [ 44, 66, 0]] ] ``` 2) ``` [ [ [ 0, 2, 3], [ 4, 0, 6], [ 7, 8, 0] ], [ [0, 12, 90], [ 3, 0, 40], [ 6, 40, 0] ], [ [ 0, 2, 3], [ 4, 0, 6], [ 44, 66, 0] ], ] ``` 3) ``` [ [[-1.0, 0.5, 0.5], [3.0, -4.0, 1.0], [0.9, 0.1, -1.0]], [[-6.0, 2.0, 4.0], [1.5, -2.0, 0.5], [3.0, 1.0, -4.0]], [[-1.0, 0.1, 0.9], [2.0, -2.5, 0.5], [0.9, 0.1, -1.0]], ] ``` If you agree @AlessandroBregoli I would also proceed to conform all the rank-3 tensors in `tests/` to the notation number 2 listed above.
meliurwen commented 2 years ago (Migrated from github.com)
Owner

We can't use ignore option into rustfmt.toml because it is not stable yet;

on the official rustfmt repo and its issues is suggested to use the inner attribute #![rustfmt::skip] instead, which is suboptimal for this use case and is also unstable on Rust, so we can't use this neither...

At this point I would give up and use cargo +nightly fmt with only ignore enabled as a non-stable option.

We can't use `ignore` option into `rustfmt.toml` because it is not stable yet; on the official `rustfmt` repo and its issues is suggested to use the inner attribute `#![rustfmt::skip]` instead, which is suboptimal for this use case and is also [unstable](https://github.com/rust-lang/rust/issues/54726) on Rust, so we can't use this neither... At this point I would give up and use `cargo +nightly fmt` with only `ignore` enabled as a non-stable option.
AlessandroBregoli (Migrated from github.com) approved these changes 2 years ago
The pull request has been merged as a5b24e9eee.
Sign in to join this conversation.
Loading…
There is no content yet.