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

Extentions to JAGS #83

Merged
merged 20 commits into from Dec 21, 2022
Merged

Extentions to JAGS #83

merged 20 commits into from Dec 21, 2022

Conversation

vandenman
Copy link
Contributor

@vandenman vandenman commented Dec 13, 2022

Fixes https://github.com/jasp-stats/INTERNAL-jasp/issues/2132
Fixes https://github.com/jasp-stats/INTERNAL-jasp/issues/2133
Fixes https://github.com/jasp-stats/INTERNAL-jasp/issues/2134

Here an example file I for testing, rename and drop .zip:

exampleNewJagsFunctionality_nostate.jasp.zip

Also, I'm aware I haven't made any unit tests. They are on my list but I figured I wanted to open the PR before the deadline!

All the comments that start with TODO for inference will be finished for the next release and won't be part of this PR.

Copy link
Member

@Kucharssim Kucharssim left a comment

Choose a reason for hiding this comment

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

Wow, so much functionality! Looks really good :)

I left just a couple of remarks for option names, otherwise the code looks really good.

inst/qml/JAGS.qml Outdated Show resolved Hide resolved
inst/qml/JAGS.qml Outdated Show resolved Hide resolved
inst/qml/JAGS.qml Outdated Show resolved Hide resolved
inst/qml/JAGS.qml Outdated Show resolved Hide resolved
inst/qml/JAGS.qml Outdated Show resolved Hide resolved
inst/qml/JAGS.qml Outdated Show resolved Hide resolved
inst/qml/JAGS.qml Outdated Show resolved Hide resolved
@vandenman
Copy link
Contributor Author

@Kucharssim I've removed the name customInference everywhere and added a few unit tests.

Copy link
Member

@Kucharssim Kucharssim left a comment

Choose a reason for hiding this comment

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

looks ok, but now i get this

Screenshot 2022-12-20 at 20 49 39

I can't find any two components sharing the same name though to help you out...

@vandenman
Copy link
Contributor Author

Good catch! there were two radio buttons with the name manual. Not sure why that isn't allowed, but it's fixed now.

@vandenman vandenman merged commit 6c7ddff into jasp-stats:master Dec 21, 2022
@vandenman vandenman deleted the customPlots branch December 21, 2022 11:15
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.

None yet

2 participants