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

ComplexField #181

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

AndyGreenwell
Copy link

Pull request for adding a ComplexField enamldef to fields.enaml for the purpose of input and display of complex numbers as well as a corresponding ComplexValidator within validator.py. The content was created by copying and slightly modifying the FloatField enamldef and FloatValidator class.

Adding ComplexField enamldef to define a field that will accept and display a complex number.  Corresponding changes made to validator.py to add a ComplexValidator.
Adding ComplexValidator that is used in conjunction with the ComplexField definition added to fields.enaml
"""
#: The minimum value allowed for the float, inclusive, or None if
#: there is no lower bound.
minimum = Typed(float)
Copy link
Member

Choose a reason for hiding this comment

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

should be Typed(complex) ?

@sccolbert
Copy link
Member

Looks mostly g2g. I made a few formatting comments and have a question about having the min/max bounds be floats instead of complex. Was there a particular reason for that?

@AndyGreenwell
Copy link
Author

No particular reason for the choices made other than just performing the minimal changes needed from the existing FloatField and FloatValidator to get a minimal level of functionality working. With min/max check, mirroring the numpy min/max behavior for complex values is probably better. I planned to submit the pull request and then wait for review and corrections. Looks like that plan worked. :)

Currently working on a project that needed input and display of complex numbers, and I really like the library! Thanks,.

@jason-sachs
Copy link

Hmm. Mixed thoughts, this is really specialized behavior, I don't have any strong objection to a ComplexField object, it just seems a bit odd for a general framework. I had a similar request for a LongField.

@AndyGreenwell
Copy link
Author

When building applications for physics or engineering, the display and
entry of complex numbers is a common use case. That is my motivation here,
as it is functionality that was missing for my current needs.

One can always break up the complex number into two components and display
the real and imaginary parts in separate fields, but that introduces
(likely unnecessary) extra operations on the
domain-specialist's/application-developer's part when their application is
processing complex numbers.

If the maintainers decide that this functionality is not necessary, then
please deny this pull request, and I will move forward with a different
implementation in my current application.

Thanks,

Andy

On Wed, Apr 1, 2015 at 10:51 AM, Jason Sachs notifications@github.com
wrote:

Hmm. Mixed thoughts, this is really specialized behavior, I don't have any
strong objection to a ComplexField object, it just seems a bit odd for a
general framework. I had a similar request
#172 for a LongField.


Reply to this email directly or view it on GitHub
#181 (comment).

@sccolbert
Copy link
Member

@AndyGreenwell I'm all for the pull request. Please update your PR to address my comments and then I'll merge it.

@jason-sachs The difference between this PR and what you were suggesting is that there is no long type in Python 3, and Python 3 will be the target going forward.

Addressing comments by @sccolbert
@sccolbert
Copy link
Member

Actually, does it even make sense to have a range on complex numbers? There's not really a standard for relative ordering of complex numbers...

@blink1073
Copy link
Contributor

Well, you could constrain on magnitude and/or phase...

@AndyGreenwell
Copy link
Author

True, complex numbers are not an ordered field (in a mathematical sense), so just imposing an arbitrary order in code might cover some cases cleanly but not others.

That said, being able to impose minimum and maximum values on the real and imaginary parts separately can be useful in some physical contexts. For example, in an electrical engineering context, one might have separate limits on the real and imaginary parts of the impedance (resistance and reactance), but end up doing computations on the complex quantity. While that example may in some ways be arguing against inclusion of a ComplexField, having a field (in a UI sense) that neatly accepts, displays, and validates a complex value is useful.

I defer to your better judgment on whether this functionality should be included.

@sccolbert
Copy link
Member

Instead of having min/max range on this validator, what about a callable property which the user could assign which would validate the value after it passes the initial string -> complex conversion?

Base automatically changed from master to main January 19, 2021 10:00
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