TomographicImaging / eqt

A number of templates and tools to develop Qt GUI's with Python effectively.
Other
2 stars 3 forks source link

PR Insert widget #109

Closed DanicaSTFC closed 7 months ago

DanicaSTFC commented 10 months ago

Closes #103 closes #105 closes #95 Closes issue https://github.com/TomographicImaging/eqt/issues/107. Partially resolves 108.

Full summary:

INSERT WIDGET CHANGES

DICTIONARY CHANGES

STATE SAVING CHANGES

OTHER CHANGES

Before first review.

INSERT WIDGET CHANGES

DICTIONARY CHANGES

STATE SAVING CHANGES

OTHER CHANGES

DanicaSTFC commented 10 months ago

TO DO: (DONE 05/12/2023)

DanicaSTFC commented 10 months ago

@lauramurgatroyd I still need to fix the example "Insert widget" and "Remove widget" and the unit tests. Could you start having a look at it because I have made a lot of changes in this PR, please?

Note after meeting (07/12/2023): for the first review, please do not look at example insert widget yet.

DanicaSTFC commented 10 months ago

(DONE ON 09/01/2024) After group discussion, we decided to change the state format to have widget number in both roles: field and label.

DanicaSTFC commented 9 months ago

After first review:

Closes issue #107. Partially resolves issue #108.

DanicaSTFC commented 8 months ago

TO DO (12/01/2024):

TO DO (19/01/2024):

casperdcl commented 8 months ago

before I submit a detailed review... some high-level points after meeting with @DanicaSTFC @lauramurgatroyd @paskino:

DanicaSTFC commented 8 months ago

@casperdcl Thank you for your commits. When I pulled them 128 tests failed and 8 passed. I do not see a red cross in the checks above. Could you look into why the checks have passed in GitHub? Also, would you like to fix the code or should I do it?

casperdcl commented 8 months ago

128 tests failed and 8 passed. I do not see a red cross in the checks above. Could you look into why the checks have passed in GitHub? Also, would you like to fix the code or should I do it?

Please do push any fixes! It might help me find the problem(s) with the tests :/

DanicaSTFC commented 8 months ago

128 tests failed and 8 passed. I do not see a red cross in the checks above. Could you look into why the checks have passed in GitHub? Also, would you like to fix the code or should I do it?

Please do push any fixes! It might help me find the problem(s) with the tests :/

test error

I fixed most errors. The screenshot shows a a recurring error in the tests. If I delete the folder they run again.

casperdcl commented 8 months ago

I fixed most errors. The screenshot shows a a recurring error in the tests. If I delete the folder they run again.

unrelated #128 lol. Issue number 128 for 128 failing tests :D

TL;DR you probably had 1 failing test which didn't clean up after itself so it causes the rest to fail.

DanicaSTFC commented 8 months ago

@casperdcl Regarding the style of the docstring I am trying to use the CIL convention (with the spaces around ":"). Ideally we would change all of them to be consistent.

casperdcl commented 8 months ago

spaces around ":"

ah ok sure https://numpydoc.readthedocs.io/en/latest/format.html#parameters no problem.