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

Add SmileGAN Plugin #191

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

zhijian-yang
Copy link

No description provided.

Copy link
Contributor

@ashishsingh18 ashishsingh18 left a comment

Choose a reason for hiding this comment

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

Thanks @zhijian-yang for putting in this PR. I added a few comments. I was not able to understand if the new files differ from the ones that are already a part of Compute Spares plugin.

self.plotCanvas.axes = self.plotCanvas.fig.add_subplot(111)
self.SPAREs = None
self.ui.stackedWidget.setCurrentIndex(0)
self.ui.factorial_progressBar.setValue(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change the name of progressBar. It says 'factorial_progressBar' and is confusing since there is no factorial computation.

from NiBAx.core.baseplugin import BasePlugin
from NiBAx.core.gui.SearchableQComboBox import SearchableQComboBox

class computeSPAREs(QtWidgets.QWidget,BasePlugin):
Copy link
Contributor

Choose a reason for hiding this comment

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

Without going through every single line, this class seems to be a copy of computespares.py. If so, we might not need to duplicate it here, we could just instantiate that one.

self.PopulateHue()


class BrainAgeWorker(QtCore.QObject):
Copy link
Contributor

Choose a reason for hiding this comment

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

This class also looks like a duplicate of the one in computeSpares.py. Can we not re-use that?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the confusion. I am adding a Smile-GAN plugin with Ahmed step by step now, so it is still in progress and it is now mostly copied from compute-spare.

Copy link
Contributor

Choose a reason for hiding this comment

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

@zhijian-yang Is this PR still a work in progress? If yes, we can convert this into a draft.
Also, if you want to re-use code from ComputeSpares class, a better approach is to derive from that class.

@zhijian-yang zhijian-yang marked this pull request as ready for review July 26, 2022 17:51
@zhijian-yang
Copy link
Author

Hi, I have completed one version of SmileGAN plugin. Please check and tell me how I should improve it. Thanks!

@ashishsingh18
Copy link
Contributor

Thanks @zhijian-yang
@AbdulkadirA Could you please take a look at this PR? I will be able to get to it late next week.

Copy link
Contributor

@AbdulkadirA AbdulkadirA left a comment

Choose a reason for hiding this comment

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

@zhijian-yang Thanks. Basic functions work as expected.

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.

3 participants