Skip to content

Plotting#54

Merged
RemDelaporteMathurin merged 21 commits into
mainfrom
plotting
Dec 2, 2024
Merged

Plotting#54
RemDelaporteMathurin merged 21 commits into
mainfrom
plotting

Conversation

@RemDelaporteMathurin
Copy link
Copy Markdown
Collaborator

@RemDelaporteMathurin RemDelaporteMathurin commented Nov 30, 2024

Needs #55

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 10 changed files in this pull request and generated no suggestions.

Files not reviewed (4)
  • bin_data.dat: Language not supported
  • all_bin_scenario.py: Evaluated as low risk
  • src/hisp/init.py: Evaluated as low risk
  • src/hisp/festim_models/mb_model.py: Evaluated as low risk
Comments skipped due to low confidence (6)

test/test_bin.py:4

  • The import statement includes FWBin, which is a valid addition for the new tests.
from hisp.bin import FWBin3Subs, FWBin2Subs, DivBin, FWBin, BinCollection, Reactor

src/hisp/bin.py:51

  • The calculation for 'low_wetted' and 'high_wetted' modes might cause a division by zero error if 'self.low_wetted_area' or 'self.high_wetted_area' is zero. Add a check to ensure these values are not zero before performing the division.
def wetted_frac(self):

src/hisp/bin.py:73

  • Ensure that 'start_point' and 'end_point' are correctly initialized before calculating the length. Add a check to verify these properties are not None.
def length(self) -> float:

make_iter_bins.py:145

  • Ensure that the columns 'R_Coord' and 'Z_Coord' exist in the 'data' DataFrame to avoid a KeyError.
bin.start_point = (data.loc[bin.index]["R_Coord"], data.loc[bin.index]["Z_Coord"])

make_iter_bins.py:150

  • Confirm that the fallback to 'bin.index = 0' in case of a ValueError is the intended behavior.
next_bin = my_reactor.get_bin(bin.index + 1)

make_iter_bins.py:156

  • Ensure that 'FW_bins' and 'Div_bins' are defined and populated correctly before this assertion.
assert len(data) == len(FW_bins.bins) + len(Div_bins.bins)

@RemDelaporteMathurin RemDelaporteMathurin marked this pull request as ready for review December 2, 2024 14:36
Copy link
Copy Markdown
Collaborator Author

@RemDelaporteMathurin RemDelaporteMathurin left a comment

Choose a reason for hiding this comment

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

Reviewed this with @kaelyndunnell in person. Merging now

@RemDelaporteMathurin RemDelaporteMathurin merged commit d2b2e34 into main Dec 2, 2024
@RemDelaporteMathurin RemDelaporteMathurin deleted the plotting branch December 2, 2024 14:37
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

Successfully merging this pull request may close these issues.

2 participants