GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Rebased #431: Default dt is 0 by bnavigator · Pull Request #490 · python-control/python-control

Conversation

@bnavigator

@bnavigator bnavigator commented Dec 31, 2020

Copy link
Copy Markdown
Contributor

Rebased version of #431. I had the updated tests already prepared in earlier versions of #438

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
bnavigator and others added 10 commits December 31, 2020 00:34
@bnavigator
Revert "make tests work with pre python-control#431 source code state…
578cfa6 
…" from python-control#438

This reverts commit 2b98769.
@sawyerbfuller @bnavigator
set default dt to be 0 instead of None.
86401d6
@sawyerbfuller @bnavigator
changes so that a default dt=0 passes unit tests. fixed code everywhe…
1ed9ee2 
…re that combines systems with different timebases
@sawyerbfuller @bnavigator
small bugfix to xferfcn._evalfr to correctly give it dt value
5f46b5b
@sawyerbfuller @bnavigator
re-incorporated lti.timebaseEqual, with a pendingDeprecationWarning
24eeb75
@sawyerbfuller @bnavigator
changes to enable package-wide default, 'control.default_dt')
0223b82
@sawyerbfuller @bnavigator
fixes to docstrings and conventions.rst to match convention of dt=0 a…
1ca9dec 
…s default system timebase
@sawyerbfuller @bnavigator
remove extraneous file
3b2ab18
@sawyerbfuller @bnavigator
new method is_static_gain() for state space and transfer function sys…
351e0f7 
…tems, refactored __init__ on both to use it
@sawyerbfuller @bnavigator
added keyword arguments to xferfcn.tf and statesp.ss to set dt
e3a0070
@bnavigator
bnavigator requested a review from sawyerbfuller December 31, 2020 01:23
This was linked to issues Dec 31, 2020
Problem with DT State-Space Response #419
Closed
use dt=0 instead of dt=None (unspecified timebase) as default option for lti systems? #422
Closed
Bugs in InterconnectedSystem: different dt's [fixed] and non-proper TFs [open] #399
Closed
@bnavigator bnavigator mentioned this pull request Dec 31, 2020
Default dt is 0 #431
Closed
@coveralls

coveralls commented Dec 31, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 87.52% when pulling 75f3aef on bnavigator:rebase-pr431 into 6ed3f74 on python-control:master.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coveralls

coveralls commented Dec 31, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 87.005% when pulling adc8166 on bnavigator:rebase-pr431 into d66f4e4 on python-control:master.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator bnavigator changed the title Rebase pr431 Rebased #431: default dt is 0 Dec 31, 2020
@bnavigator bnavigator changed the title Rebased #431: default dt is 0 Rebased #431: Default dt is 0 Dec 31, 2020
bnavigator added 2 commits December 31, 2020 03:06
@bnavigator
update tests for dt=0, fix the constructor checks for missing dt
404b23c
@bnavigator
remove duplicate is_static_gain
8b82850
@bnavigator
bnavigator force-pushed the rebase-pr431 branch from d83ade4 to 8b82850 Compare December 31, 2020 02:08
murrayrm
murrayrm approved these changes Dec 31, 2020
View reviewed changes

@murrayrm murrayrm left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I look through the changes and these look good to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@murrayrm
Merge branch 'master' into rebase-pr431
b9dc8df
@murrayrm murrayrm mentioned this pull request Dec 31, 2020
Bugs in InterconnectedSystem: different dt's [fixed] and non-proper TFs [open] #399
Closed
sawyerbfuller
sawyerbfuller reviewed Dec 31, 2020
View reviewed changes
Comment thread control/tests/discrete_test.py
TransferFunction.__add__(tsys.siso_tf1c, tsys.siso_tf1d)
with pytest.raises(ValueError):
TransferFunction.__add__(tsys.siso_tf1d, tsys.siso_tf2d)
with pytest.raises(ValueError):

@sawyerbfuller sawyerbfuller Dec 31, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I assume these are removed because these tests are already performed above in eg ‘test_timenase_conversion’?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@bnavigator bnavigator Dec 31, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

They don't raise an error anymore. You had an equivalent change in #431. dt=0.1 and dt=True are compatible now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@sawyerbfuller sawyerbfuller Dec 31, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I see, makes sense!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sawyerbfuller
sawyerbfuller reviewed Dec 31, 2020
View reviewed changes
Comment thread control/xferfcn.py
try:
dt = args[0].dt
except NameError: # pragma: no coverage
except AttributeError:

@sawyerbfuller sawyerbfuller Dec 31, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🤙

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sawyerbfuller
sawyerbfuller approved these changes Dec 31, 2020
View reviewed changes

@sawyerbfuller sawyerbfuller left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good to me!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bnavigator
add test for dt in arg and kwarg
75f3aef
@bnavigator

bnavigator commented Dec 31, 2020

Copy link
Copy Markdown
Contributor Author

Added one more simple test in order to not uncover a line. The double dt resolver is documented behavior and thus should be checked.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
bnavigator merged commit d1502f4 into python-control:master Dec 31, 2020
This was referenced Dec 31, 2020
Release 0.8.4 and 0.9.0 sequencing #451
Closed
generate error for tf2ss of non-proper transfer function #492
Merged
@bnavigator
bnavigator deleted the rebase-pr431 branch January 5, 2021 20:13
@murrayrm murrayrm added this to the 0.9.0 milestone Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sawyerbfuller sawyerbfuller sawyerbfuller approved these changes
@murrayrm murrayrm murrayrm approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9.0

Development

Successfully merging this pull request may close these issues.

use dt=0 instead of dt=None (unspecified timebase) as default option for lti systems? Problem with DT State-Space Response Bugs in InterconnectedSystem: different dt's [fixed] and non-proper TFs [open]

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@bnavigator @coveralls @murrayrm @sawyerbfuller
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.