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 a convenience subroutine for getting a simplified radical expression #932

Merged

Conversation

Alex-Jordan
Copy link
Contributor

This macro was previously in the PCC section of the OPL macros. This addition is a tool we have used in some problem files. It's simplest on my end to leave it here, but I acknowledge it may be a better fit somewhere else...

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

I agree that this would probably fit better somewhere else. This method could be useful in many situations where the contextLimitedRadical.pl is not needed. I am not sure where exactly to advise you to put this though.

macros/contexts/contextLimitedRadical.pl Outdated Show resolved Hide resolved
@Alex-Jordan
Copy link
Contributor Author

This is old code that I just submitted as it was. I should have reviewed it for inefficiencies or other ugliness. I'll make changes, probably exactly as you suggest, soon.

@Alex-Jordan Alex-Jordan force-pushed the update-contextLimitedRadical branch from f36100e to 156d335 Compare October 10, 2023 23:32
@Alex-Jordan
Copy link
Contributor Author

I committed your suggestion plus one minor cleanup. Still not sure where to put this other than here. It returns a Formula() so it relies on MathObjects.

@Alex-Jordan Alex-Jordan force-pushed the update-contextLimitedRadical branch from 156d335 to 8faa63b Compare December 2, 2023 20:34
@Alex-Jordan Alex-Jordan force-pushed the update-contextLimitedRadical branch from 8faa63b to 0e7cf9f Compare January 10, 2024 05:31
@Alex-Jordan Alex-Jordan force-pushed the update-contextLimitedRadical branch from 0e7cf9f to 7aec500 Compare January 25, 2024 17:57
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

Let's merge this. I don't think anyone has found a better place for it.

@Alex-Jordan Alex-Jordan force-pushed the update-contextLimitedRadical branch from 7aec500 to 19675f8 Compare February 7, 2024 20:55
Copy link
Member

@drdrew42 drdrew42 left a comment

Choose a reason for hiding this comment

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

Yeah, let's put it in.

@pstaabp pstaabp merged commit 64b5543 into openwebwork:develop Feb 7, 2024
2 checks passed
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.

4 participants