addition of a more realistic, less nice, example signal #3

mKlapwijk · 2020-09-11T11:55:08Z

No description provided.

…. Addition of discarding of timesteps based on selected valued in the figure, which is then automatically applied to the signal graph. TST graph is now also separated into a stationary range and discarded part. Addition of moving average in signal graph

Nanoseb · 2020-09-15T11:56:04Z

pyTST/core.py

+
+ moving_average : array of float
+ moving average based on the used step size


moving_average is not a parameter for load_data_array, so this docstring shouldn't be there.

Nanoseb · 2020-09-15T11:56:20Z

pyTST/core.py

 """

 self.signal_array = signal_array
 if time_array is None:
 self.time_array = (np.array(range(len(self.signal_array)))+1)*tstep
 else:
 self.time_array = time_array
+ self.moving_average = np.array(signal_array.size)


is this needed?

You are right, it can be removed

Nanoseb · 2020-09-15T12:01:07Z

pyTST/core.py

@@ -170,7 +189,8 @@ def import_from_txt(self, filename):
 self.u95_array = timedata[:, 1]
 self.mean_array = timedata[:, 2]

- def plot(self, filename=None, show_cursor=True):
+ def plot(self, filename=None, fileFormat='.png',show_cursor=True,\
+ selectTimeDiscard=True,step_size=10):


try to be consistent with the naming convention, ie: here don't use camel case since everything else is snake case (see https://www.python.org/dev/peps/pep-0008)
and with this it is no longer a filename but only a prefix

I never heard about the terms camel vs snake case. Good point

Nanoseb · 2020-09-15T12:01:31Z

pyTST/core.py

@@ -181,19 +201,23 @@ def plot(self, filename=None, show_cursor=True):
 if provided, the plot will be exported to filename

 show_cursor : bool, optional
- True if a cursor is ploted. Double clicking on the plot will move it
+ True if a cursor is plotted. Double clicking on the plot will move it
+ selectTimeDiscard: bool, optional


add newline between comments here

Nanoseb · 2020-09-15T12:02:50Z

pyTST/core.py

+ ax0.loglog(self.step_time_array[:self.step_time_array.size - (self.step_time_array.size-index)], self.u95_array[:(self.step_time_array.size-index-1):-1], color='C0')
+ ax0.loglog(self.step_time_array[self.step_time_array.size - (self.step_time_array.size-index):], self.u95_array[(self.step_time_array.size-index-1)::-1], color='C1')


these lines are too long and should have at least one line break.
also self.step_time_array.size is used a lot, storing it in a variable will make it easier to read

Nanoseb · 2020-09-15T12:04:43Z

pyTST/core.py

 pyplot.draw()
+ global indexGlobal 


is a global variable really needed. These should be avoided, and are 90% of the time not useful. Here you can just use self.current_index and update that in the onclick function I think

(and again camel case when it should be snake case

In principle I completely agree! However I couldn't get it to work without it (I spend quite some time on it yesterday), but I'll try your suggestion

Nanoseb · 2020-09-15T12:54:09Z

pyTST/core.py

 text.set_text('u95={:2e}\nt={}'.format(min_u95, discard_time))
- print("t={}, mean={:e} ± {:e}".format(discard_time, self.mean_array[-index-1], min_u95))
+ print("t={}, mean={:e} ± {:e}".format(discard_time, self.mean_array[-index-1], min_u95)) 
+ return index


why do you return index?

Nanoseb · 2020-09-15T13:03:35Z

pyTST/core.py

+ # Plot input signal data
+ fig1, ax1 = pyplot.subplots()
+
+ if selectTimeDiscard:
+ idx = (self.step_time_array.size-indexGlobal)*step_size
+ else:
+ idx = index

+ ax1.axvline(self.time_array[idx], color='k', lw=0.8, ls='--', alpha=0.6)

+ ax1.plot(self.time_array[0:idx], self.signal_array[0:idx], color='C1',linewidth=2,label='discarded time')
+ ax1.plot(self.time_array[idx:], self.signal_array[idx:], color='C0',linewidth=2,label='stationary region')
+ ax1.plot(self.time_array,self.moving_average, color='r',label='moving average')

+ ax1.set_xlabel("$t / $s",fontsize=16)
+ ax1.set_ylabel("$\phi$",fontsize=16)
+ ax1.legend(fontsize=16)


I feel like this shouldn't be in pyTST but in the example script. Have self.plot() return the index (that should be called something else, my bad for picking a ugly variable name), and then this can be outside. As it is an example of how ppl can use the TST data to 'understand' their signal. But you don't always want/need to plot your input signal again. And also, you often want to have more freedom in plotting it (like changing the axis names, title etc.)

All in all, I think .plot() should return the index, and fig, ax variables. That way the user can edit the TST plot, and use index to plot the signal.

Nanoseb · 2020-09-15T13:05:37Z

pyTST/core.py

+ if filename is None:
+ pyplot.show()
+ else:
+ pyplot.show()


shouldn't have .show() here. You want a way to export the tst plot without having any user interaction (for scripting etc.). Or show a user defined component independent of filename

Nanoseb · 2020-09-15T13:09:22Z

pyTST/core.py

+def moving_average(y, n=10) :
+ y_padded = np.pad(y, (n//2, n-1-n//2), mode='edge')
+ y_smooth = np.convolve(y_padded, np.ones((n,))/n, mode='valid') 
+ return y_smooth


As discussed earlier, is it really useful considering self.mean_array already exist.

Nanoseb · 2020-09-15T13:16:03Z

So ;)
I made some comments. Tell me if you want to fix those before merging, otherwise when I have more time I will manually pick and rewrite what is needed.

I think the end goal should be to have 2 different behaviours for the .plot function:

Non interactive, you get the TST plot (either .show or .filesave()), and that's it (so a bit like the current status) -> .plot should return index (with an other name) and fig, ax for maximum freedom in processing the data afterwards
Interactive, where you see both your signal and the TST plot side by side and double clicking on the TST plot updates the colors on both of them (seeing what is being discarted and what not etc.)

Right now, having the signal after the TST plot inside pyTST.plot, is a bit of a mix between the two, and I feel like it constrain too much the end user.

Nanoseb · 2020-10-27T11:34:02Z

has partially been implemented in master already, and #5 implements the rest.

mKlapwijk added 3 commits September 11, 2020 13:54

addition of a more realistic, less nice, example signal

38c9b92

Nanoseb reviewed Sep 15, 2020

View reviewed changes

mKlapwijk added 4 commits September 22, 2020 14:22

remove excessive files

bc600e8

modifications after review by SL

acfabf8

modifications after review by SL

e39f897

make TST plot also partly orange, partly blue, based on start up effect

4b9f1dd

Nanoseb closed this Oct 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

addition of a more realistic, less nice, example signal #3

addition of a more realistic, less nice, example signal #3

mKlapwijk commented Sep 11, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

mKlapwijk Sep 15, 2020

Nanoseb Sep 15, 2020

mKlapwijk Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

mKlapwijk Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb Sep 15, 2020

Nanoseb commented Sep 15, 2020

Nanoseb commented Oct 27, 2020


		moving_average : array of float
		moving average based on the used step size

		ax0.loglog(self.step_time_array[:self.step_time_array.size - (self.step_time_array.size-index)], self.u95_array[:(self.step_time_array.size-index-1):-1], color='C0')
		ax0.loglog(self.step_time_array[self.step_time_array.size - (self.step_time_array.size-index):], self.u95_array[(self.step_time_array.size-index-1)::-1], color='C1')

addition of a more realistic, less nice, example signal #3

addition of a more realistic, less nice, example signal #3

Conversation

mKlapwijk commented Sep 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nanoseb commented Sep 15, 2020

Nanoseb commented Oct 27, 2020