-
Notifications
You must be signed in to change notification settings - Fork 6
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
Widget to customize number of seeds to generate #79
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good at a glance, modulo my notes. I'd be happy to incorporate this in a new release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems about ready. (I don't have a test suite like I should, but if it's working for you...)
If you want to be super cool, since you have a working dev environment and I actually don't right now, I can set you up to push a release candidate to PyPI, and if that works successfully I wouldn't mind letting you make the full release.
dashboard/checkit/outcome.py
Outdated
@@ -50,7 +50,8 @@ def to_dict(self,regenerate=False): | |||
|
|||
def preview_exercises(self): | |||
preview_json = os.path.join(self.build_path(),"preview.json") | |||
sage(self,preview_json,preview=True,images=True) | |||
# hardcode: previews generate 10 seeds | |||
sage(self,preview_json,amount=10,preview=True,images=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amount=10
will be ignored by the sage script (see next comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yep, this works. (I was worried that the signature wouldn't match but it's ok because we handle a default amount in wrapper/init.py.)
@@ -206,15 +206,15 @@ if len(sys.argv) >= 4: | |||
if sys.argv[3].lower() == "preview": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we ignore the amount if we're set to "preview"
. So I think it's better to simply omit the amount=10
earlier as it's misleading (it will use the default 1000 instead, but that's happily ignored here).
OR previews could support variable #s of seeds (maybe you really want to preview just 1 seed, or 100). But that's more work for minimal benefit?
FYI I've been feeling kinda poorly since Saturday but I'll do more stuff
when I can.
…On Fri, Jul 21, 2023, 11:16 AM Steven Clontz ***@***.***> wrote:
***@***.**** requested changes on this pull request.
This seems about ready. (I don't have a test suite like I should, but if
it's working for you...)
If you want to be super cool, since you have a working dev environment and
I actually don't right now, I can set you up to push a release candidate to
PyPI, and if that works successfully I wouldn't mind letting you make the
full release.
------------------------------
In dashboard/checkit/outcome.py
<#79 (comment)>:
> @@ -50,7 +50,8 @@ def to_dict(self,regenerate=False):
def preview_exercises(self):
preview_json = os.path.join(self.build_path(),"preview.json")
- sage(self,preview_json,preview=True,images=True)
+ # hardcode: previews generate 10 seeds
+ sage(self,preview_json,amount=10,preview=True,images=True)
amount=10 will be ignored by the sage script (see next comment)
------------------------------
In dashboard/checkit/wrapper/wrapper.sage
<#79 (comment)>:
> @@ -206,15 +206,15 @@ if len(sys.argv) >= 4:
if sys.argv[3].lower() == "preview":
here we ignore the amount if we're set to "preview". So I think it's
better to simply omit the amount=10 earlier as it's misleading (it will
use the default 1000 instead, but that's happily ignored here).
OR previews could support variable #s of seeds (maybe you really want to
preview just 1 seed, or 100). But that's more work for minimal benefit?
—
Reply to this email directly, view it on GitHub
<#79 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACDYL6WGJBDBZDX6UJ5U3YTXRK2PFANCNFSM6AAAAAA2TBK64I>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
No rush - I'm happy to help you make a 0.2.7 release with this feature if that's more convenient for you, or do it myself once someone else makes the same request. |
I am more or less back. I wonder if before we push out a 0.2.7 official
release, we may also want to attempt to get latex markdown preview working
again. Do you know where / when it broke?
…On Wed, Jul 26, 2023 at 9:29 AM Steven Clontz ***@***.***> wrote:
No rush - I'm happy to help you make a 0.2.7 release with this feature if
that's more convenient for you, or do it myself once someone else makes the
same request.
—
Reply to this email directly, view it on GitHub
<#79 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACDYL6WBOJQSWPDQVGY2HNDXSEZUHANCNFSM6AAAAAA2TBK64I>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Ah. That's caused by using non-standard Jupyter that CoCalc supported, but probably not JupyterLab et al. Let me see something... |
My hunch is that changing the |
Unfortunately that change in html.xsl is no immediate dice. |
This commit makes a new
IntText
widget on the dashboard so you can customize the number of seeds to generate.The value of this widget passes into
output.generate_exercises()
, which now has a new signature including an amount to generate.generate_exercises()
feeds this value to the sage command inwrapper/__init__.py
, and the indexing ofsys.argv
inwrapper.sage
is modified accordingly.(Please feel free to ignore the version number change in
checkit/__init__.py
-- I put that in there so I could make sure I was using the local dev copy of dashboard instead of the global one.)