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: The alerts package #2646

Merged
merged 49 commits into from
Dec 2, 2019
Merged

Conversation

dvbridges
Copy link
Contributor

@dvbridges dvbridges commented Oct 10, 2019

  • On import, the alerts catalogue id loaded into the catalogue variable as a dict
  • ErrorHandler created as an app attribute, like stdoutFrame
  • On compile script, the ErrorHandler and stdOutFrame are assigned to the stderr and stdout stream, respectively.
  • On script compile, psyexpCompile calls integrity check method in base component and code component
  • IntegrityCheck method calls alerttool tests
  • When alert is found, the alerttool tests call _alerts.alert function, with code, component, strformat, and any tracebacks
  • The alert function creates an AlertEntry object for each alert
  • The alert function sends AlertEntry object to receiveAlert method in ErrorHandler, which should be on the standard error stream.
  • ErrorHandler stores each alert object in its alerts attribute
  • When flushed, the ErrorHandler, sends the alerts list to the alertLog container in the _alerts module.
  • When users open ProjectInfo, the alertLog is used to populate the new alert panel

- MasterLog no longer stores alerts, but creates alert logs,
  although folder needs a location
- Changes to yaml catalogue keys
Pathlib is in the Python 3.4 standard library after PEP 428 acceptance.
It needs installing for Python 3.3 or older.
Should fail for Python 2 until pathlib is set as a dependency
This tidies up the component modules
This enables the master log to be created in each project folder. Also
reformats the alert messages to include component name and type, with
relevant checks. This provides more useful messages.
The timing test currently only tests whether the start time occurs
after the end time of the component. Includes relevant checks for size
and position attributes, and catches for errors that occur when evaluating
 a string containing a variable. Currently, parameters containing
variables cannot be tested because the value of the variable is unknown.
Also sends output file of psyexpCompile test to temp directory.
Passes locally, but may fail on Travis
This enables catalogue messages to be formatted so they are more
informative. Also refactors catalogue as fewer entries are required
because of the new ability to format messages.
Now, code component syntax is tested for Python and JS each time a script
is compiled. Any errors are allowed to pass but alerts are formatted
with informative information, such as which tab contains the error.
The information is sent to the stdoutFrame and saved in the alerts
log folder. Also adds string formatting values to the size, position and
time checks,  checks time values can be tested correctly.
- Adds test for disabled components
- Renames alert module
- Alert module no longer has log classes, instead alerts are called using
the alert function.
- Adds error handler for receiving errors from the error stream
- In progress alert dialog created as a target for alerts. This will be
a panel added to the Project Info dialog
- Integrity check added to psyexpCompile
- Tests adapted to new alert system
- Alerts from errorhandler now show in ProjectInfo each time it opens
- Alerts now stored in separate list from errors in ErrorHandler
- removes alerts generated when tests fail due to variable use
- renames receiveWarning method in ErrorHandler to receiveAlert
- AlertLog container created to feed alert panel
- Removes setting of stderr, can be done using Builders setStandardStream
- Stores original stderr in app, as with stdout
- Corrects Builder subprocess to write to stream, rather than variable
The error handler is flushed when it writes errors, so calls to write
interrupt the reporting of Alerts. The current solution is to change
calls to write from error stream to logging module, and this preservers
alerts. There were also issues with capturing the error stream, solved
here.
Syntax checks would not pass because the Param needed to pass the value to
the compile function.
For now, we only need to capture the error stream for the error handler.
The output of its prints are directed to the stdout, so for seeing warnings
it only matters where that is pointing to. In future, it may be good to
send outputs and errors to the runner frame only, one source for all
information.
@dvbridges
Copy link
Contributor Author

@peircej , apart from the travis error, this pull request passes.

@codecov-io
Copy link

codecov-io commented Oct 10, 2019

Codecov Report

Merging #2646 into master will decrease coverage by 0.28%.
The diff coverage is 76.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2646      +/-   ##
==========================================
- Coverage   44.28%   43.99%   -0.29%     
==========================================
  Files         234      241       +7     
  Lines       42153    44600    +2447     
  Branches     7286     7589     +303     
==========================================
+ Hits        18666    19622     +956     
- Misses      21501    22969    +1468     
- Partials     1986     2009      +23
Impacted Files Coverage Δ
psychopy/experiment/_experiment.py 50.76% <ø> (-0.39%) ⬇️
psychopy/app/coder/coder.py 28.45% <ø> (+0.18%) ⬆️
psychopy/app/stdOutRich.py 87.75% <ø> (+0.88%) ⬆️
psychopy/experiment/routine.py 87.76% <ø> (+0.88%) ⬆️
psychopy/visual/aperture.py 74.28% <ø> (+1.35%) ⬆️
psychopy/app/pavlovia_ui/toolbar.py 67.92% <0%> (-2.67%) ⬇️
psychopy/experiment/components/unknown/__init__.py 87.8% <100%> (+0.3%) ⬆️
psychopy/app/_psychopyApp.py 37.86% <100%> (+0.17%) ⬆️
psychopy/alerts/__init__.py 100% <100%> (ø)
psychopy/experiment/components/_base.py 62.25% <100%> (+0.37%) ⬆️
... and 83 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8db8521...6202191. Read the comment docs.

@coveralls
Copy link

coveralls commented Oct 10, 2019

Coverage Status

Coverage increased (+0.9%) to 48.055% when pulling 6202191 on dvbridges:globalErrorHandling into 8db8521 on psychopy:master.

@lgtm-com
Copy link

lgtm-com bot commented Oct 10, 2019

This pull request introduces 3 alerts and fixes 1 when merging bb017ce into 8db8521 - view on LGTM.com

new alerts:

  • 3 for Unused import

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

Including AlertLog now iterable and indexable
They were in place for future, will use when needed.
@lgtm-com
Copy link

lgtm-com bot commented Nov 6, 2019

This pull request introduces 1 alert and fixes 1 when merging 7bbaf75 into c63332a - view on LGTM.com

new alerts:

  • 1 for __iter__ method returns a non-iterator

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

@lgtm-com
Copy link

lgtm-com bot commented Nov 7, 2019

This pull request fixes 1 alert when merging 2f3e5a3 into bb5e637 - view on LGTM.com

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

@lgtm-com
Copy link

lgtm-com bot commented Nov 7, 2019

This pull request fixes 1 alert when merging cebe934 into bb5e637 - view on LGTM.com

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

@lgtm-com
Copy link

lgtm-com bot commented Nov 8, 2019

This pull request fixes 1 alert when merging af02af5 into e937f5a - view on LGTM.com

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

Main change is the use of separate alert files for each alert.
Alert files support restructured text for implementation in web help pages
Includes alertCategories list as yaml.
@lgtm-com
Copy link

lgtm-com bot commented Nov 26, 2019

This pull request introduces 2 alerts and fixes 1 when merging 695339a into 81fb15f - view on LGTM.com

new alerts:

  • 2 for Property in old-style class

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

@lgtm-com
Copy link

lgtm-com bot commented Nov 27, 2019

This pull request fixes 1 alert when merging 6202191 into 81fb15f - view on LGTM.com

fixed alerts:

  • 1 for Missing call to __init__ during object initialization

@peircej peircej merged commit 5b396c8 into psychopy:master Dec 2, 2019
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