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

Adds parameters and experimental parameters to the project widget #49

Merged
merged 20 commits into from
Nov 6, 2024

Conversation

alexhroom
Copy link
Collaborator

This PR fixes #40 by adding parameters and experimental parameters to the project widget.

@alexhroom alexhroom marked this pull request as ready for review October 31, 2024 15:59
@alexhroom alexhroom force-pushed the 40-param-tables branch 3 times, most recently from 0d35c40 to 24f3b77 Compare November 1, 2024 11:02
Copy link
Contributor

@StephenNneji StephenNneji left a comment

Choose a reason for hiding this comment

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

Looks good so far please see comments

  • Also when in edit mode, I think we might need a horizontal scrollbar but I would prefer if we could get it to fit without one. It is also possible to edit the column where the delete button is meant to be in for the substrate roughness while I don't think it would lead to any issues would be nice to disable edit in that column.Screenshot 2024-11-05 135723
  • I cannot edit the mu and sigma values
  • It would be nice to have the fit header toggle the checkboxes similar to https://gist.github.com/StephenNneji/14bfc4e7a322ec89df7d30847fbf19b3?permalink_comment_id=2588841#gistcomment-2588841 but we could do later if useful

rascal2/widgets/project.py Outdated Show resolved Hide resolved
rascal2/widgets/project.py Outdated Show resolved Hide resolved
rascal2/ui/view.py Outdated Show resolved Hide resolved
rascal2/widgets/project.py Outdated Show resolved Hide resolved
rascal2/widgets/project.py Outdated Show resolved Hide resolved
rascal2/widgets/project.py Outdated Show resolved Hide resolved
rascal2/widgets/project.py Outdated Show resolved Hide resolved
@alexhroom
Copy link
Collaborator Author

@StephenNneji your comment on not being able to edit the mu and sigma values: they're set to only be editable if prior_type is set to gaussian (which I think we agreed on in a meeting). otherwise they should be greyed out, or at least they are on my machine? they don't look to be on yours

@StephenNneji
Copy link
Contributor

@StephenNneji your comment on not being able to edit the mu and sigma values: they're set to only be editable if prior_type is set to gaussian (which I think we agreed on in a meeting). otherwise they should be greyed out, or at least they are on my machine? they don't look to be on yours

I can confirm that it works, I was testing on Ubuntu and it does not grey out the cells. Also the dropdown feels finicky, it doesn't enable the mu and sigma except I press enter or click another cell. The inf spinbox does nothing when the value is infinity and I press the down button, I am not sure the appropriate behaviour for an inf spinbox but I think if possible the button should be disabled if it does nothing or jump to a reasonable start value maybe?

@alexhroom alexhroom force-pushed the 40-param-tables branch 2 times, most recently from b374729 to ae9c51d Compare November 6, 2024 11:06
ruff format

fixed typo

fixed typo
@alexhroom alexhroom requested review from StephenNneji and removed request for StephenNneji November 6, 2024 11:55
Copy link
Contributor

@StephenNneji StephenNneji left a comment

Choose a reason for hiding this comment

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

Almost there, A few cosmetic things

  • I think the widgets on top need some layout tweaks especially in edit mode so the widget is closer to the labels
  • Can we make the header title case ?

Screenshot 2024-11-06 134902

rascal2/widgets/project/models.py Show resolved Hide resolved
rascal2/widgets/project/project.py Outdated Show resolved Hide resolved
rascal2/core/commands.py Show resolved Hide resolved
Copy link
Contributor

@StephenNneji StephenNneji left a comment

Choose a reason for hiding this comment

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

Good Job, Thanks for the hard work on this one.

I am getting a lots of user warning when I change the fit, please have a look if it something we need to fix or just silence

/home/stephen/miniconda3/envs/rascal2/lib/python3.12/site-packages/pydantic/main.py:364: UserWarning: Pydantic serializer warnings:
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[Background]` but got `ClassList` - serialized value may not be as expected
  Expected `list[definition-ref]` but got `ClassList` - serialized value may not be as expected
  Expected `list[Resolution]` but got `ClassList` - serialized value may not be as expected
  Expected `list[CustomFile]` but got `ClassList` - serialized value may not be as expected
  Expected `list[Data]` but got `ClassList` - serialized value may not be as expected
  Expected `Union[list[Layer], list[AbsorptionLayer]]` but got `ClassList` - serialized value may not be as expected
  Expected `list[DomainContrast]` but got `ClassList` - serialized value may not be as expected
  Expected `Union[list[Contrast], list[ContrastWithRatio]]` but got `ClassList` - serialized value may not be as expected
  return self.__pydantic_serializer__.to_python(

@alexhroom
Copy link
Collaborator Author

@StephenNneji this seems to be a python-RAT bug in the new version when you use Project.model_dump(): see the following code produces the same warning

from RATapi import Project
p = Project()
p.model_dump()

so i've created the issue RascalSoftware/python-RAT#90 for it

@alexhroom alexhroom merged commit 80c671d into RascalSoftware:main Nov 6, 2024
10 checks passed
@alexhroom alexhroom deleted the 40-param-tables branch November 6, 2024 15:36
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.

Create widget for the parameter and experimental parameter for the project window
2 participants