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 :init to ratio #373

Merged
merged 2 commits into from
Apr 24, 2019
Merged

Add :init to ratio #373

merged 2 commits into from
Apr 24, 2019

Conversation

Affonso-Gui
Copy link
Member

Current:

eus$ (instance ratio :init)
;; eus 0 error: cannot find method :init in (send #:inst60 :init)
eus$ (instantiate ratio)
;; eus 0 error: number expected in euserror

Proposed:

eus$ (instance ratio :init 1 2)
1/2

Not sure if all the makefile changes are necessary (although seem to be sufficient).

@@ -1,4 +1,6 @@
(defmethod ratio
(:prin1 (&optional (strm t))
(format strm "~d/~d" numerator denominator))
Copy link
Member

Choose a reason for hiding this comment

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

@Affonso-Gui why you removed prin1 ?, please also update doc/test

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed prin1 because

  1. printratio is already doing the job https://github.com/euslisp/EusLisp/blob/master/lisp/c/printer.c#L219-L226
  2. extnum.l and its :prin1 method are currently not being loaded in euslisp.

Copy link
Member Author

Choose a reason for hiding this comment

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

@k-okada Added test code. There seems to be no ratio-related documentation to be updated. We can add some if that is the case, preferentially in #359 (am planning on working on that PR this week)

@Affonso-Gui
Copy link
Member Author

Tests fail/pass as expected

@Affonso-Gui
Copy link
Member Author

@k-okada Please review

@k-okada k-okada merged commit da192ce into euslisp:master Apr 24, 2019
@k-okada
Copy link
Member

k-okada commented Apr 24, 2019

@Affonso-Gui do we have a document for this?

@Affonso-Gui Affonso-Gui deleted the init-ratio branch April 25, 2019 02:24
@Affonso-Gui
Copy link
Member Author

@k-okada Currently we have no documentation for ratio in jmanual. Should I add some (including the :init method from this PR)?

@k-okada
Copy link
Member

k-okada commented Apr 25, 2019

Should I add some (including the :init method from this PR)?

yes please

@Affonso-Gui
Copy link
Member Author

@k-okada Created at #381

k-okada added a commit that referenced this pull request Jun 7, 2019
Add extended-number doc
add :init to complex number c.f #373
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.

2 participants