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

let Transducer(word) output a word #18123

Closed
dkrenn opened this issue Apr 3, 2015 · 27 comments
Closed

let Transducer(word) output a word #18123

dkrenn opened this issue Apr 3, 2015 · 27 comments

Comments

@dkrenn
Copy link
Contributor

dkrenn commented Apr 3, 2015

Make

sage: W = Words([0,1])
sage: inverter(W([0,1,0]))
word: 101
sage: inverter(words.FibonacciWord([0,1]))
word: 1011010110110101101011011010110110101101...

work.

See also #18118.

Depends on #18118
Depends on #18114
Depends on #15267
Depends on #16133

CC: @videlec @cheuberg @sagetrac-skropf

Component: finite state machines

Keywords: sd66, transducer, words

Author: Daniel Krenn

Branch/Commit: 64ee3e8

Reviewer: Clemens Heuberger

Issue created by migration from https://trac.sagemath.org/ticket/18123

@dkrenn dkrenn added this to the sage-6.6 milestone Apr 3, 2015
@dkrenn
Copy link
Contributor Author

dkrenn commented Apr 3, 2015

Dependencies: #18118

@dkrenn
Copy link
Contributor Author

dkrenn commented Apr 6, 2015

Branch: u/dkrenn/fsm/words

@dkrenn
Copy link
Contributor Author

dkrenn commented Apr 6, 2015

Author: Daniel Krenn

@dkrenn
Copy link
Contributor Author

dkrenn commented Apr 6, 2015

Last 10 new commits:

9bf4d02iter_process_simple: docstring and small renaming of parameter
9376b18improved doctests (using words)
3d3039bMerge branch 'fsm/iter_process-simple' into fsm/words
7d8605eadd option automatic_output_type
9c09323make `__call__` work nicly with infinite words
d28af2fminor code simplification
192671dmention new option in docstring of __call__
894eb39docstring __call__: reorganize examples
afb0178__call__: examples on words added
c6340acinclude words-examples in top of file

@dkrenn

This comment has been minimized.

@dkrenn
Copy link
Contributor Author

dkrenn commented Apr 6, 2015

Changed dependencies from #18118 to #18118, #18114, #15267

@dkrenn
Copy link
Contributor Author

dkrenn commented Apr 6, 2015

Commit: c6340ac

@cheuberg
Copy link
Contributor

Work Issues: merge current version of #18114

@cheuberg
Copy link
Contributor

Changed branch from u/dkrenn/fsm/words to u/cheuberg/fsm/words

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2015

Changed commit from c6340ac to 1d7354f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

0d4e1daMerge #18114 due to renamed method (commit f209b0fec910bcae1f5998ba6b61c0371433ee34)
1d7354fTrac #18123: Adapt to current version of #18114

@cheuberg
Copy link
Contributor

Changed work issues from merge current version of #18114 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2015

Changed commit from 1d7354f to b3e699e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 18, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

a6e0fe2Trac #18118: Reviewer Patch: Fix ReST formatting error
137cbf9Trac #18118: Reviewer Patch: Add doctest
f059bc2Trac #18123: Merge latest version of #18118
6819d78Trac #18123: Reviewer patch: show input before computing output in doctests
b3e699eTrac #18123: Reviewer Patch: Language and ReST formatting error

@cheuberg
Copy link
Contributor

Reviewer: Clemens Heuberger

@cheuberg
Copy link
Contributor

comment:10

IMHO .iter_process should return an iterator in every case. Wouldn't it be cleaner to remove automatic_output from .iter_process alltogether and move this convenience option to .process or even only to the convenience method __call__?

Further comments:

  • Introductory example: The sentence "Here processing works with iterators in background." is unclear to me.
  • code of __call__: in the branch if ... input_tape.is_finite() == False:, the check if not 'automatic_output_type' in kwargs seems to be superfluous because it is set before.
  • __call__: Claims that automatic_output_type is set, but it is not.
  • documentation of .process: What is the difference in the results of automatic_output_type=False and automatic_output_type=None?
  • Please include doctests for automatic_output_type.
  • Code of iter_process: IMHO setting kwargs['format_output'] once and using it once, but then again guarded by if_automatic_output_type is some kind of overkill and also some kind of abuse of that parameter.
  • For simplicity, I'd prefer to eliminate the possibility automatic_output=None: The magic that format_output is investigated in order to determine automatic_output could as well be moved to __call__. I can hardly imagine a use case of calling .process directly with automatic_output=None.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 28, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

e2fcf3bdeprecation warning (#16132) removed
0039d7dTransducer.cartesian_product: deprecation warning deleted
5f45124removed also deprecated variable
4a83285Merge tag '6.8' into fsm/cartesian-product-intersection-no-deprecation
24f7b3bTrac #16106: remove further occurrences of FSMOldCodeTransducerCartesianProduct
ae13509Trac #16133: Merge #16106
6b3172eTrac #16133: Three docstrings: command instead of description
1a30b06Trac #16133: remove three further instances of FSMOldProcessOutput
8765992Trac #18123: Merge #16133 to resolve conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 28, 2015

Changed commit from b3e699e to 8765992

@cheuberg
Copy link
Contributor

Changed dependencies from #18118, #18114, #15267 to #18118, #18114, #15267. #16133

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2015

Changed commit from 8765992 to c70b19a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 13, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

4bc8bc6add additional examples to show difference between 'simple'-option and not using it
f00e904seealso block in _iter_process_simple_
a28221bminor simplifiction of code and add a comment explaining resetting of output
29a54c3doctests for error conditions and rephrase these errors to get more information when raised
7d9e481Trac #18118: Additional doctest
3ecc012Trac #18118: Rephrase error message.
c70b19aMerge #18118 to resolve merge conflict

@dkrenn
Copy link
Contributor Author

dkrenn commented Sep 14, 2015

Changed branch from u/cheuberg/fsm/words to u/dkrenn/fsm/words

@dkrenn
Copy link
Contributor Author

dkrenn commented Sep 14, 2015

Changed commit from c70b19a to 64ee3e8

@dkrenn
Copy link
Contributor Author

dkrenn commented Sep 14, 2015

comment:15

Replying to @cheuberg:

IMHO .iter_process should return an iterator in every case. Wouldn't it be cleaner to remove automatic_output from .iter_process alltogether and move this convenience option to .process or even only to the convenience method __call__?

.iter_process returns an iterator, but it can be of any type (depending on input type/class) which supports iterating. Thus did not change it.

Further comments: [...]

All done; see commits.


Last 10 new commits:

5d59d90adapt code after merge to pass doctests
e439bf2Trac #15267: Reviewer patch: Use ZZ(..., base=2)
6e47b1dminor rephrase doc: remove one "only"
80d8798rename write_in_every_step and rephrase doc
7c9a164rephrase/extend doc of process_iterator_class
b580917Merge remote-tracking branch 'origin/u/dkrenn/fsm/languages' into fsm/words
da369d0#18123 comment 10, bullet 1: sentence deleted.
c02c375#18123 comment 10, bullet 2: delete unnecessary check
2301c1aTrac #18123 comment 10, bullet 3, 4, 6, 7: remove automatic_output_type=None
64ee3e8Trac #18123 comment 10, bullet 5: add additional doctests

@cheuberg
Copy link
Contributor

comment:17

modifications are fine, doctests pass, documentation ok.

@vbraun
Copy link
Member

vbraun commented Sep 17, 2015

Changed dependencies from #18118, #18114, #15267. #16133 to #18118, #18114, #15267, #16133

@vbraun
Copy link
Member

vbraun commented Sep 18, 2015

Changed branch from u/dkrenn/fsm/words to 64ee3e8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants