Reactance

[avgagliardo]

PR for the reactance module.

-Use of Lambda functions can be found around lines 126-143 -Use of Dict Comprehension can be found on 90 -Use of List Comprehensions are frequent -Use of the unittest module's assert objects are used in unit-tests.py

[avgagliardo]

TODO maybe add plots

[avgagliardo]

module gracefully terminated linting process on a non-severe exit code

[laserlab]

Check also the automated test for linting @avgagliardo you got a 7.8/10 so far

[avgagliardo]

Check also the automated test for linting @avgagliardo you got a 7.8/10 so far

I did see that, but many of the complaints are due to choices that I made intentionally. There's a couple of docstrings that seem to be missing, I fixed those which put the linting score over an 8/10.

The other complaints I am not entirely convinced are worth the "code smell" that fixing them would introduce. Should not be an issue make the changes, but I can point to style rules that suggest ignoring linting complaints in specific cases-- trading readability just to have code lint well can become its own unique detriment.

• Linting complaints such as too many positional arguments could be fixed with passing lists or dicts, but explicit is often better than implicit and so I left many of the parameters in their simple and familiar form, instead of composing them into more opaque objects.
• Failure to adhere to camel casing variables when it detracts from the readability, such as in cases where it makes sense to use variables that have subscripts, since that resembles the notation that human are used to recognizing.
• And finally line too long complaints (by a few characters) is usually automatically fixed by soft-wrapping instead of splitting line. Alternatively, I could use much smaller and less descriptive variable names, but that again comes at the cost of human readability. EDIT: I fixed the comments that were too long, which meant culling some of the inline ones entirely but that is fine.

I am comfortable with the linting complaints, but like I said I am happy to change them if @laserlab or @zbpetersbuf deems it necessary.

[laserlab]

@zbpetersbuf please review asap

[laserlab]

@avgagliardo Would you mind splitting the .gitignore and the documentation/testDrivenDevelopment.md out into a separate PR. That way I can merge it without review.

[avgagliardo]

@avgagliardo Would you mind splitting the .gitignore and the documentation/testDrivenDevelopment.md out into a separate PR. That way I can merge it without review.

Just sent PR #37 over.

[laserlab]

@avgagliardo Would you mind splitting the .gitignore and the documentation/testDrivenDevelopment.md out into a separate PR. That way I can merge it without review.

Just sent PR #37 over.

Can you remove the files from this PR to avoid merging issues

[avgagliardo]

removed the split files

[laserlab]

Btw. @avgagliardo Your code lints to 8.17/10. So while pretty good, not perfect. I also know these are minor issues but it might not hurt to fix them if you have time.

[laserlab]

Check also the automated test for linting @avgagliardo you got a 7.8/10 so far

I did see that, but many of the complaints are due to choices that I made intentionally. There's a couple of docstrings that seem to be missing, I fixed those which put the linting score over an 8/10.

The other complaints I am not entirely convinced are worth the "code smell" that fixing them would introduce. Should not be an issue make the changes, but I can point to style rules that suggest ignoring linting complaints in specific cases-- trading readability just to have code lint well can become its own unique detriment.

• Linting complaints such as too many positional arguments could be fixed with passing lists or dicts, but explicit is often better than implicit and so I left many of the parameters in their simple and familiar form, instead of composing them into more opaque objects.
• Failure to adhere to camel casing variables when it detracts from the readability, such as in cases where it makes sense to use variables that have subscripts, since that resembles the notation that human are used to recognizing.
• And finally line too long complaints (by a few characters) is usually automatically fixed by soft-wrapping instead of splitting line. Alternatively, I could use much smaller and less descriptive variable names, but that again comes at the cost of human readability. EDIT: I fixed the comments that were too long, which meant culling some of the inline ones entirely but that is fine.

These are all very valid points, but for pure educational reasons it would makes sense to fix them for this homework. I do not claim that all those linting issues are worth fixing in general, but just for this homework to pass the automated test.

[avgagliardo]

I'd never try and represent my code as perfect, I was just saying that I believe I had hit a sweet spot between human readability and linter-induced madness.

After killing the linting complaints the code is much more arcane and difficult to read, but it passes with a 10/10. Additionally I noticed that it would alternate between wanting camel cased variables and snake cased variables (in the same instances) between different linting runs, so the linter itself might be a little bit confused about what it wants. That would make sense since there are slightly different linting standards.

[laserlab]

I know. I'm happy to adjust linting requirements for the next homeworks and I would be happy to include yours and others suggestions. However since it's the first time we do the check I opted for standard options to make it less complex.

[laserlab]

I'd never try and represent my code as perfect, I was just saying that I believe I had hit a sweet spot between human readability and linter-induced madness.

After killing the linting complaints the code is much more arcane and difficult to read, but it passes with a 10/10. Additionally I noticed that it would alternate between wanting camel cased variables and snake cased variables (in the same instances) between different linting runs, so the linter itself might be a little bit confused about what it wants. That would make sense since there are slightly different linting standards.

On the plus side you now have that pretty ✅

[avgagliardo]

@zbpetersbuf this PR is ready for review btw