Skip to content

Add output format for integrations#46

Merged
BwKodex merged 34 commits intomasterfrom
add_output_format_for_integrations
Oct 24, 2023
Merged

Add output format for integrations#46
BwKodex merged 34 commits intomasterfrom
add_output_format_for_integrations

Conversation

@BwKodex
Copy link
Copy Markdown
Contributor

@BwKodex BwKodex commented May 26, 2023

No description provided.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 3, 2023

Codecov Report

Merging #46 (d1471e8) into master (5ce5add) will decrease coverage by 1.04%.
The diff coverage is 55.55%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   68.23%   67.20%   -1.04%     
==========================================
  Files          52       55       +3     
  Lines        1700     1857     +157     
==========================================
+ Hits         1160     1248      +88     
- Misses        540      609      +69     
Files Coverage Δ
src/pyskindose/constants.py 100.00% <100.00%> (ø)
src/pyskindose/db_connect.py 100.00% <100.00%> (ø)
.../pyskindose/helpers/calculate_rotation_matrices.py 100.00% <100.00%> (ø)
src/pyskindose/plotting/create_geometry_plot.py 43.75% <ø> (+4.86%) ⬆️
src/pyskindose/rdsr_normalizer.py 100.00% <100.00%> (ø)
src/pyskindose/rdsr_parser.py 82.25% <ø> (-0.29%) ⬇️
tests/integrationtests/conftest.py 100.00% <100.00%> (ø)
...tegrationtests/test_calculate_rotation_matrices.py 100.00% <100.00%> (ø)
tests/integrationtests/test_corrections.py 87.50% <100.00%> (ø)
tests/integrationtests/test_rdsr_normalizer.py 100.00% <100.00%> (ø)
... and 12 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@BwKodex BwKodex marked this pull request as ready for review October 10, 2023 21:21
@MaxHellstrom MaxHellstrom self-requested a review October 12, 2023 11:07
class HumanPhantomOutput:
"""Create and handle a patient phantom data for output into a dict or JSON-string."""

def __init__(self, phantom: Phantom):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

missing docstring entries

beam.
"""

def __init__(self, data_norm: pd.DataFrame):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

missing docstring entries

Table : Phantom
The treatment table as an instance of the Phantom class
Pad : Phantom
The treatment table pad as an instance of the Phantom class
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same goes here, and so on...

Idea: lets skip this and then address language issues in a separate branch?

patient : Phantom
An instance of the Phantom class that represents the patient
table : Phantom
An instance of the Phantom class that represents the treatment table
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

shouldnt it be "A phantom that represents the treatment table"?

It is clear from the "patient: Phantom" that the parameter table is of instance Phantom

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It can be considered clear that it is an instance of the phantom class from that but I believe it is more clear to read the text when we specify it this way. I think I would be more confused of reading the sentence when specifying the table as a phantom. But I'll leave i t up to you to decide :)

@BwKodex BwKodex linked an issue Oct 12, 2023 that may be closed by this pull request
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this notebook output should be removed, right? It seems like the notebook output is online.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have removed it now

@BwKodex BwKodex merged commit 437cf9c into master Oct 24, 2023
@BwKodex BwKodex deleted the add_output_format_for_integrations branch October 24, 2023 07:46
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.

PySkinDose output structure

3 participants