Skip to content
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.

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

T105 overview manual test #318

Merged
merged 27 commits into from
Jul 29, 2024
Merged

T105 overview manual test #318

merged 27 commits into from
Jul 29, 2024

Conversation

cbender98
Copy link
Collaborator

@cbender98 cbender98 commented Jun 26, 2024

Please check the following before creating the pull request (PR):

  • Did you run automatic tests?
  • Did you run manual tests?
  • Is the code provided in the PR still backwards compatible to previous SIMPA versions?

List any specific code review questions
I added new dependencies. Please check if this is fine.

List any special testing requirements

Additional context (e.g. papers, documentation, blog posts, ...)

Provide issue / feature request fixed by this PR

Fixes #105

Copy link
Collaborator

@Holzwarth69126 Holzwarth69126 left a comment

Choose a reason for hiding this comment

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

Some minor refactoring is necessary. Afterwards the functionality has to be checked,

requirements.txt Outdated Show resolved Hide resolved
simpa_tests/__init__.py Outdated Show resolved Hide resolved
simpa_tests/__init__.py Outdated Show resolved Hide resolved
simpa_tests/manual_tests/generate_overview.py Outdated Show resolved Hide resolved
simpa_tests/manual_tests/generate_overview.py Outdated Show resolved Hide resolved
simpa_tests/manual_tests/generate_overview.py Show resolved Hide resolved
simpa_tests/manual_tests/generate_overview.py Outdated Show resolved Hide resolved
simpa_tests/manual_tests/generate_overview.py Outdated Show resolved Hide resolved
simpa_tests/manual_tests/generate_overview.py Outdated Show resolved Hide resolved
@frisograce
Copy link
Collaborator

Please add information about that functionality in the readme.

@Holzwarth69126
Copy link
Collaborator

We (@kdreher @frisograce ) were not able to generate an HTML file on two different workstations. The .md file and figures were created and looked ok. The HTML file was created but seemed to be empty.

@cbender98 could you please look into that? Either there is a bug somewhere or the documentation should be updated.

@TomTomRixRix
Copy link
Collaborator

We (@kdreher @frisograce ) were not able to generate an HTML file on two different workstations. The .md file and figures were created and looked ok. The HTML file was created but seemed to be empty.

@cbender98 could you please look into that? Either there is a bug somewhere or the documentation should be updated.

When running the manual test generate_overview.py on my computer the HTML file with content was created without errors. Of course I had to run pip install . to install the dependencies.

@cbender98 cbender98 closed this Jul 26, 2024
@cbender98 cbender98 reopened this Jul 26, 2024
@frisograce
Copy link
Collaborator

Checked this all on a clean version of simpa, and it all worked. Only one problem where the reconstucted image for KWaveAcousticForwardConvenienceFunction isn't working, but I dont know if this could be a me problem.

@cbender98
Copy link
Collaborator Author

cbender98 commented Jul 26, 2024

  • put new requirements in optional requirements
  • add feature to create own reference figure folder (with logging simpa version)

@cbender98
Copy link
Collaborator Author

ready to merge. above requested changes have been addressed. @kdreher @jgroehl you can a have a final look over it.

@kdreher kdreher merged commit 270e5d2 into develop Jul 29, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants