Closed carolinafernandezp closed 1 month ago
Attention: Patch coverage is 80.72289%
with 16 lines
in your changes are missing coverage. Please review.
Project coverage is 92.55%. Comparing base (
54ddd40
) to head (78ab67d
). Report is 49 commits behind head on master.:exclamation: Current head 78ab67d differs from pull request most recent head d046764. Consider uploading reports for the commit d046764 to get more accurate results
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
can you share what the example outputs in the PR description?
can you share what the example outputs in the PR description?
Updated! I'm not getting the expected ratios (1:2, α:β) so I'm looking into that now.
whatever happened to this PR ? Was this close to merge? Is this something this year's GSoC student can work on?
whatever happened to this PR ? Was this close to merge? Is this something this year's GSoC student can work on?
Hi, I'm going to work on getting this merged but I think it's definitely something this year's GSoC student can expand on.
@carolinafernandezp you seem to have rebase conflicts ... might be easiest to create a new branch from latest master, cherry pick your commits on to that branch, rename that branch to optimize_rhythmic
and force push.
The build is failing because of the GUI example ... @gtdang @kmilo9999 how best to address this? maybe we could separate the GUI examples into a separate build using Github actions?
We can just get rid of those GUI docs from the build. New docs for the GUI will just be markdown to reduce the complexity and fragility of the notebook builds.
How do we guarantee that the examples will run as expected for the user? unit tests?
How do we guarantee that the examples will run as expected for the user? unit tests?
Exactly this, after going through the numerous problems with rendering the GUI tutorials, having hand-made tutorials with less extensive testing seems like the better option than having absolutely no tutorials due to an updated package
alright, go for it ... please remove it so we can move ahead with @carolinafernandezp 's PR
@carolinafernandezp we've finally fixed the documentation issues, if you have the bandwidth in the next week or so would you want to do a quick zoom call to rebase this together and go ahead and merge it?
Also note for myself to close #176 once this is merged
Sounds great @ntolley, how is Thursday the 18th? Any time would work for me
Just tested this locally and everything is running great, thanks a bunch @carolinafernandezp this is a super useful contribution!!
Thanks @ntolley and everyone that helped!👏🏻
The goals are:
Example output
![image](https://github.com/jonescompneurolab/hnn-core/assets/51917715/a20a6a25-43a1-4cc9-8f7b-13d7eb9884ff)
Reversing the ratios
obj_fun
as a callable.