Skip to content

Bugfix/plot without friction - #55

Open
EleniSmyrniou wants to merge 11 commits into
masterfrom
bugfix/plot_without_friction
Open

Bugfix/plot without friction#55
EleniSmyrniou wants to merge 11 commits into
masterfrom
bugfix/plot_without_friction

Conversation

@EleniSmyrniou

Copy link
Copy Markdown
Collaborator

In this issue the plot is not generated if there is not a value of friction and/or friction number. This is not the functionality we want. What I changed is that the tip resistance ( and depth) is the only parameter that is required for the plottting

@sonarqubecloud

sonarqubecloud Bot commented Feb 4, 2026

Copy link
Copy Markdown

@EleniSmyrniou EleniSmyrniou self-assigned this Feb 4, 2026
@sonarqubecloud

sonarqubecloud Bot commented Apr 1, 2026

Copy link
Copy Markdown

Comment thread tests/test_plot_cpt.py Outdated
Comment thread tests/test_plot_cpt.py Outdated
Comment thread tests/test_plot_cpt.py Outdated
Comment thread tests/test_plot_cpt.py Outdated

result = plot_cpt.check_data_availability_for_plotting(cpt, "friction_nbr")

assert result is True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you also correct the rest of the asserts to assert result, instead of assert result is True

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed all

Comment thread tests/test_plot_cpt.py Outdated
Comment on lines +589 to +601
def test_check_data_availability_qc_raises_value_error_without_tip(self):
"""
Test that check_data_availability_for_plotting raises ValueError with correct message
when tip data is not available for qc plotting
"""
cpt = BroXmlCpt()
cpt.tip = None

with pytest.raises(
ValueError,
match="Tip data is not available for plotting, this is required for plotting.",
):
plot_cpt.check_data_availability_for_plotting(cpt, "qc")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this test is a duplicate

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed

Comment thread tests/test_plot_cpt.py Outdated
"""
cpt = BroXmlCpt()

result = plot_cpt.check_data_availability_for_plotting(cpt, "unknown_key")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this behaviour is a bit strange, why would a unknown key return True

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes good point

Comment thread geolib_plus/plot_cpt.py
pdf.close()


def check_data_availability_for_plotting(cpt: AbstractCPT, key) -> bool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think its better to invert the if statements here, i.e. if key == "something" and cpt.something is not None, return true. but default value false. so unavailable keys also return false

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ok changed the function

Comment thread tests/test_plot_cpt.py

# Check that a PDF file was created
pdf_files = list(output_dir.glob("*.pdf"))
assert len(pdf_files) > 0, "No PDF file was created"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this comment doesnt seem right

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed

EleniSmyrniou and others added 3 commits June 23, 2026 11:13
Co-authored-by: aronnoordam <51492202+aronnoordam@users.noreply.github.com>
Co-authored-by: aronnoordam <51492202+aronnoordam@users.noreply.github.com>
Co-authored-by: aronnoordam <51492202+aronnoordam@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

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