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

ENH: Preparations for releasing v1.3.0 #30

Open
wants to merge 146 commits into
base: master
Choose a base branch
from

Conversation

ankostis
Copy link

@ankostis ankostis commented Oct 9, 2019

Preparations for baking a new release & last-days fixes.

  • enh(TCs): Doctest all code; enabled many assertions that were just print-outs
    in v1.2.4.
  • FIX(ops): operation.__repr__() was crashing when not all arguments had been set - a condition frequtnly met during debugging session.
  • enh(net): Annotate operation exceptions with ExecutionPlan to aid debug sessions,
    inspired by @syamajala's 309338340.
  • doc: add CHANGES.rst changelog, with flowcharts comparing versions 1.2.4 <-->1.3..0.
  • doc: add API reference chapter.
  • enh: build, site & documentation for all new features.
  • drop(build): sphinx_rtd_theme is the default for sphinx now.

GraphkitFlowchart-v1 3 0


I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

were writing in text-mode in PY3. and failing as encoding error.
Tokens work as usual while solving the DAG but
they are never assigned any values to/from the operation's functions.

+ TC included.
+ Docs updated.\+ Added `modifiers` superclass to facilitate identification code.
+ refactored FunctionalOperation._compute().
+ _norm_kwargs:
  + use isinstance() instead of type-equality checks,
    to support new modier classes;
    + avoid excessive dict searches with local vars.
not really needed, better be explicit which modifier is searched.
receiving partial inputs, needed for other operations.
+ The x2 TCs added just before are now passing.
NOTE dict are not deterministic in <PY3.6.
So this commit would not improve determinism
in those pythons.

+ build: add `boltons` dependency for ndexedSet.
+ doc: mark all set usage if affect determinism.
+ e.g. see reproducibility problem in yahoo#14.
needed to refactor the new pruning algorithm.

- expected 2 TCs fail for yet-to-be-solved yahoo#24 & yahoo#25 bugs.
override intermediate data.

More changes only for newer pruning TCs:
+ refact(test): rename graph-->netop vars for results of compose(),
  to avoid of `graph.net.graph`.
+ Explain failure modes in v1.2.4 & this merged branch (yahoo#19 + yahoo#23).
not after compose().

+ All TCs pass ok.
+ NOTE this is not yet what is described in yahoo#21.
to pass +TC checking DeleteInst vary when inputs change.

- x4 TCs still failing, and need revamp of dag-solution.
+ Read the next doc-only commit to understand changes.
+ Renamed:
  + net.steps --> net.execution_plan.
  + (old)compile() --> _build_execution_plan()
  + _find_necessary_steps() --> (new)compile() + _solve_dag()
    compile() became the master function invoking _solve_dag &
    _build-execution_plan(), and do the caching.
+ refact(compute()): extract common tasks from sequential/parallel.
+ refact show_layers() to allow full-print, geting also string
  (not just printing), and using custom classes for representation.
+ Raise AssertionError when invalid class in plan.
  it's a logic error, not a language type-error.
Probaly unreported bug in v1.2.4 for '_neccessary_steps_cache`.
+ Pruning behaves correctly also when outputs given;
  this happens by breaking incoming provide-links
  to any given intermedediate inputs.
+ Unsatisfied detection now includes those without outputs
  due to broken links (above).
+ Remove some uneeded "glue" from unsatisfied-detection code,
  leftover from previous compile() refactoring.
+ Renamed satisfiable --> satisfied.
+ Improved unknown output requested raise-message.
+ x3 TCs PASS, x1 in yahoo#24 and the first x2 in yahoo#25.
- 1x TCs in yahoo#25 still FAIL, and need "Pinning" of given-inputs
  (the operation MUST and MUST NOT run in these cases).
numpy was used just for its assert_raise
@ankostis
Copy link
Author

ankostis commented Oct 10, 2019

PR huyng#1 Done.

@ankostis
Copy link
Author

Dear Huy, If you the original GH-repo is salvagable, i suggest the do any discussion eithetr on that repo, or on the commits themselves, so as not to proliferate information to multiple repos.

so as to politely print IndexedSets in those attributes.
but still annotate edges with optional edge data-attribute.

Reverted bc Operation must not know its network, to belong to more than one.
Also the `Operation.net` contradicted `NetwotkOperation.net`;
the later indeed is the network it wraps
(not the net it is part of).
+ homegine titles
+ fix: autodoc renders nothing without :special-members:
+ enh(net): mark dag-edges with sideffects;
+ enh(plot): plot sideffect inks as such;
+ enh(plot): update legend;
+ doc(plot): move legend text along with the code producing it.
+ doc: stray image fixes.
if built without cleaing, stray artifacts might make it into wheel.
and when giving `pytest --lf` i get:

_____________________________________ ERROR collecting setup.py _____________________________________
/usr/lib/python3.7/distutils/fancy_getopt.py:233: in getopt
    opts, args = getopt.getopt(args, short_opts, self.long_opts)
/usr/lib/python3.7/getopt.py:93: in getopt
    opts, args = do_longs(opts, args[0][2:], longopts, args[1:])
/usr/lib/python3.7/getopt.py:157: in do_longs
    has_arg, opt = long_has_args(opt, longopts)
/usr/lib/python3.7/getopt.py:174: in long_has_args
    raise GetoptError(_('option --%s not recognized') % opt, opt)
E   getopt.GetoptError: option --lf not recognized
as debug aid (was doing that for plans already).

+ enh: annotate also networks.
@ankostis
Copy link
Author

This is considered done.

Review moved to #31. Will close with 1.3.0

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.

5 participants