Fix save class

[DidierRLopes]

Fixes #2053

Even though we exit/reset we want to save class so that if we need the attributes on one of the levels above, we don't get an issue.

I also noticed that we weren't saving a class unless it only received the queue as a parameter. I added a comment that explains what's happening in there. There's no right or wrong about it - but it's important to understand why it's there.

[colin99d]

Fixes #2053

Even though we exit/reset we want to save class so that if we need the attributes on one of the levels above, we don't get an issue.

I also noticed that we weren't saving a class unless it only received the queue as a parameter, so I removed that constraint of only having 1 argument. @colin99d do you remember why you added it in the first place? Can't think of a reason, why it would have been necessary and want to make sure I'm not missing something important here.

I think the reason I did that was if someone sent new arguments it would load those, and not keep the old ones.

For example. With the new menu if you type in the following commands:
python terminal.py
stocks
load a
fa
q
load b
fa
data
You get data for a and not b.

[DidierRLopes]

Fixes #2053
Even though we exit/reset we want to save class so that if we need the attributes on one of the levels above, we don't get an issue.
I also noticed that we weren't saving a class unless it only received the queue as a parameter, so I removed that constraint of only having 1 argument. @colin99d do you remember why you added it in the first place? Can't think of a reason, why it would have been necessary and want to make sure I'm not missing something important here.

I think the reason I did that was if someone sent new arguments it would load those, and not keep the old ones.

For example. With the new menu if you type in the following commands: python terminal.py stocks load a fa q load b fa data You get data for a and not b.

I see.

But in that case we never save any class that has arguments to be init with, right?

[colin99d]

Fixes #2053
Even though we exit/reset we want to save class so that if we need the attributes on one of the levels above, we don't get an issue.
I also noticed that we weren't saving a class unless it only received the queue as a parameter, so I removed that constraint of only having 1 argument. @colin99d do you remember why you added it in the first place? Can't think of a reason, why it would have been necessary and want to make sure I'm not missing something important here.

I think the reason I did that was if someone sent new arguments it would load those, and not keep the old ones.
For example. With the new menu if you type in the following commands: python terminal.py stocks load a fa q load b fa data You get data for a and not b.

I see.

But in that case we never save any class that has arguments to be init with, right?

They way I remember it working is that if any changes happen it loads again but if nothing changes it stays the same.

[DidierRLopes]

Fixes #2053
Even though we exit/reset we want to save class so that if we need the attributes on one of the levels above, we don't get an issue.
I also noticed that we weren't saving a class unless it only received the queue as a parameter, so I removed that constraint of only having 1 argument. @colin99d do you remember why you added it in the first place? Can't think of a reason, why it would have been necessary and want to make sure I'm not missing something important here.

I think the reason I did that was if someone sent new arguments it would load those, and not keep the old ones.
For example. With the new menu if you type in the following commands: python terminal.py stocks load a fa q load b fa data You get data for a and not b.

I see.
But in that case we never save any class that has arguments to be init with, right?

They way I remember it working is that if any changes happen it loads again but if nothing changes it stays the same.

The way I'm reading this is that if the controller you call has more than 1 input (self.queue) then it always generates a new menu. Which from my testing it's what was always happening.

When you remove that condition ==1 is when it actually saves it. But then the tricky part is that if something changes above, you don't capture that.

@piiq @jmaslek what's your opinion on this?

[colin99d]

When you remove that condition ==1 is when it actually saves it. But then the tricky part is that if something changes above, you don't capture that.

@piiq @jmaslek what's your opinion on this?

One thing we could do is save the args and kwargs sent the last time in the dict. If they are the same we load if not we generate a new one.

[DidierRLopes]

When you remove that condition ==1 is when it actually saves it. But then the tricky part is that if something changes above, you don't capture that.
@piiq @jmaslek what's your opinion on this?

One thing we could do is save the args and kwargs sent the last time in the dict. If they are the same we load if not we generate a new one.

They are already loaded and saved. You can access the saved using controllers[PATH].kwarg assuming that the kwargs have the same name as the attributes. Like here:

[DidierRLopes]

@colin99d @jmaslek I removed that additional thing, and just added more comments around what we had. This should be good to merge now.

[piiq]

I guess we need to mock get_coingecko_id for the tests to pass, right @Chavithra ?

[DidierRLopes]

I guess we need to mock get_coingecko_id for the tests to pass, right @Chavithra ?

We are good here @piiq