Skip to content

Fixing two winding transformer remark #573

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Apr 14, 2022
Merged

Fixing two winding transformer remark #573

merged 7 commits into from
Apr 14, 2022

Conversation

@staudtMarius
Copy link
Member

@staudtMarius staudtMarius commented Apr 5, 2022

Resolves #381

@staudtMarius staudtMarius added the documentation Improvements or additions to documentation label Apr 5, 2022
@staudtMarius staudtMarius added this to the Version 3.0 milestone Apr 5, 2022
@staudtMarius staudtMarius self-assigned this Apr 5, 2022
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@codecov
Copy link

codecov bot commented Apr 5, 2022 •
edited
Loading

Codecov Report

Merging #573 (e9288a7) into dev (6dd159c) will increase coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                dev     #573      +/-   ##
============================================
+ Coverage     77.88%   77.92%   +0.04%     
- Complexity     2148     2150       +2     
============================================
  Files           275      275              
  Lines          8493     8504      +11     
  Branches        807      807              
============================================
+ Hits           6615     6627      +12     
+ Misses         1485     1484       -1     
  Partials        393      393
Impacted Files Coverage Δ
.../models/input/thermal/CylindricalStorageInput.java 86.66% <0.00%> (-1.67%) ⬇️
...tamodel/models/input/system/type/WecTypeInput.java 72.00% <0.00%> (ø)
...ctory/typeinput/Transformer3WTypeInputFactory.java 100.00% <0.00%> (ø)
...n/java/edu/ie3/datamodel/models/StandardUnits.java 96.55% <0.00%> (+0.55%) ⬆️
...s/input/connector/type/Transformer3WTypeInput.java 73.52% <0.00%> (+1.47%) ⬆️
...s/input/connector/type/Transformer2WTypeInput.java 76.00% <0.00%> (+2.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca733d3...e9288a7. Read the comment docs.

@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@staudtMarius
Separated units "impedance", "impedance_per_length", "admittance" and…
5271e85 
… "admittance_per_length".

Added standard units for:
- resistance
- reactance
- resistance_per_length
- reactance_per_length

- conductance
- susceptance
- conductance_per_length
- susceptance_per_length

Changed old units for "impedance", "impedance_per_length", "admittance" and "admittance_per_length" into the newly added standard units.
@staudtMarius
Some minor changes through spotless
dd1842a
@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
@staudtMarius staudtMarius marked this pull request as ready for review April 8, 2022 07:47
danielfeismann
danielfeismann reviewed Apr 11, 2022
View reviewed changes
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work, this looks all good to me. Please also adapt for the test.

  • GridTestData.groovy
  • ConnectorValidationUtilsTest.groovy
  • ValidationUtilsTest.groovy

staudtMarius
staudtMarius commented Apr 12, 2022
View reviewed changes
Copy link
Member Author

@staudtMarius staudtMarius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adapted the mentioned tests

danielfeismann
danielfeismann reviewed Apr 14, 2022
View reviewed changes
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staudtMarius: Thank you, I think you missed the ADMITTANCE_PER_LENGTH at GridTestData.groovy (Lines 322 and 323). And the CI isn't going through. Please have a look.
grafik

@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
staudtMarius
staudtMarius commented Apr 14, 2022
View reviewed changes
Copy link
Member Author

@staudtMarius staudtMarius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have replaced the two units. Also CI now goes through.

danielfeismann
danielfeismann previously approved these changes Apr 14, 2022
View reviewed changes
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for your work!

@sonarqubegithubprchecks

This comment has been minimized.

Sign in to view
danielfeismann
danielfeismann approved these changes Apr 14, 2022
View reviewed changes
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work!

@sonarqubegithubprchecks
Copy link

sonarqubegithubprchecks bot commented Apr 14, 2022

Passed

Analysis Details

0 Issues

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell0 Code Smells

Coverage and Duplications

  • 100 percent coverage100.00% Coverage (77.10% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.60% Estimated after merge)

Project ID: edu.ie3:PowerSystemDataModel

View in SonarQube

@danielfeismann danielfeismann merged commit d457b89 into dev Apr 14, 2022
@danielfeismann danielfeismann deleted the ms/#381-fixing-two-winding-remark branch April 14, 2022 09:41
@danielfeismann danielfeismann mentioned this pull request Apr 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danielfeismann danielfeismann danielfeismann approved these changes

Assignees

@staudtMarius staudtMarius

Labels

documentation Improvements or additions to documentation

Projects

None yet

Milestone

Version 3.0

Development

Successfully merging this pull request may close these issues.

Fix a remark in the type model of a two winding transformer

3 participants

@staudtMarius @danielfeismann