Skip to content

Commit

Permalink
Fixed is_market_listed() for multi pane charts where the first pane i…
Browse files Browse the repository at this point in the history 
…sn't a price chart

Signed-off-by: timelyart <[email protected]>
  • Loading branch information
@timelyart
timelyart committed Dec 13, 2022
1 parent 09edd6f commit ad88770
Showing 1 changed file with 13 additions and 7 deletions.
20 changes: 13 additions & 7 deletions tv/tv.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,13 @@
btn_indicator_dialog_ok='#overlap-manager-root button[name="submit"]',
active_chart_asset='div.chart-container.active div.pane-legend-line.main div.pane-legend-title__description > div',
active_chart_interval='div[id="header-toolbar-intervals"] div[class*="isActive"] > div > div',
chart_container='div.chart-container div.chart-gui-wrapper canvas:nth-child(2)',
# chart_container='div.chart-container div.chart-gui-wrapper canvas:nth-child(2)',
chart_container='table.chart-markup-table',
# User Menu
btn_user_menu='button.tv-header__user-menu-button--logged',
btn_logout='button[data-name="header-user-menu-sign-out"]',
active_widget_bar='div.widgetbar-page.active',
price_axis='td.price-axis-container > div > div',
price_axis='div[class^="price-axis-currency-label-wrapper"] > div:nth-child(1) > div:nth-child(1) > span[class^="price-axis-currency-label-text"]',
)

class_selectors = dict(
Expand Down Expand Up @@ -1589,9 +1590,11 @@ def is_market_listed(browser):
"""
listed = False
try:
price_axis = find_element(browser, css_selectors['price_axis'], visible=True, except_on_timeout=False)
if price_axis:
listed = True
elements = find_elements(browser, css_selectors['price_axis'], except_on_timeout=False)
for element in elements:
if element.text != '':
listed = True
break
except StaleElementReferenceException as e:
log.info(e)
return is_market_listed(browser)
Expand Down Expand Up @@ -4241,7 +4244,6 @@ def back_test_strategy_symbol(browser, inputs, properties, symbol, strategy_conf

for chart_index in range(number_of_charts):
# move to the correct chart
# charts = find_elements(browser, "div.chart-container")
charts = find_elements(browser, css_selectors["chart_container"])
next_chart_clicked = False
while not next_chart_clicked:
Expand Down Expand Up @@ -4496,7 +4498,11 @@ def get_strategy_statistic(browser, key, previous_elements):
log.debug(e)
pass
except Exception as e:
log.exception(e)
if e.args[0] == 'Timeout waiting for has_gone_stale':
log.debug(e)
pass
else:
log.exception(e)

el = find_element(browser, css, By.CSS_SELECTOR, False, False, 1)
if not el:
Expand Down

19 comments on commit ad88770

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

@timelyart the very first time you load a chart evaluating div[class^="price-axis-currency-label-wrapper"] > div:nth-child(1) > div:nth-child(1) > span[class^="price-axis-currency-label-text"] on a multi-pane layout it will return an empty string even when the symbol is valid. Only when you disable/enable the strategy, will it populate with a value. We had exactly the same problem when using the price axis to read the currency and, therefor switched to reading that from the properties tab.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah. Yes, I remember now. I'll change it to what you suggested earlier.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

After doing some more testing.... the currency symbol seem to show up now even if it is the first chart, regardless of it being a multi chart and/or multi pane layout.

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

After doing some more testing.... the currency symbol seem to show up now even if it is the first chart, regardless of it being a multi chart and/or multi pane layout.

Just tested and on a multi pane it still doesn't show up for me.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

It doesn't come up in the DOM for you?

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't come up in the DOM for you?

The DOM element does come up, but it has no text value (initially). Only when you toggle the strategy on/off the value gets populated.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

How odd... it seems to work fine for me. I cleared the cache and opened a layout with 3 charts and 2 panes per chart (price + MACD).
All the MACD panes have an empty text value but all the price panes have a currency immediately (in my case BTC).

Maybe it depends on the asset class? What is the initial ticker on your chart?

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm checking crypto assets and this is a common pattern across all pairs.

Upon loading the chart div[class^="price-axis-currency-label-wrapper"] > div:nth-child(1) > div:nth-child(1) > span[class^="price-axis-currency-label-text"] evaluates to <span class="price-axis-currency-label-text-SfCx6ur0"></span>.

After toggeling the strategy on/off it evaluates to <span class="price-axis-currency-label-text-SfCx6ur0">BUSD</span> as expected.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Strange.

Even though I can no longer reproduce this issue myself, I'll search for the 'invalid symbol' text instead per your earlier proposal.
It is a shame, as searching for something that shouldn't be there is less efficient than searching for something that should be there.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

I found a solution in between where only for the first market Kairos will search for Invalid symbol and the other markets it will search for the currency on the price axis.

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

I found a solution in between where only for the first market Kairos will search for Invalid symbol and the other markets it will search for the currency on the price axis.

I'm pretty sure that's not going to work. The price axis value stays empty upon changing the market. The only solution that works on my end is toggling the strategy on/off. But that's even less efficient.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah. I never had this issue that severe. I only remember it from when the chart got opened but changing markets did work.

You are running MacOs, correct?

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. I never had this issue that severe. I only remember it from when the chart got opened but changing markets did work.

The deceiving thing is that once you've got it populated it works from that moment on across all charts. But the only way (I have found) to populate it is by turning the strategy on/off.

You are running MacOs, correct?

Correct.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Would a refresh help?

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

Would a refresh help?

Nope unfortunately not.

I'm surprised that you're not experiencing this. Are you sure that you have set the indicator in the second pane as an external indicator in the strategy of your first pane? So these two panes need to be "connected". When they aren't you don't run into the issue.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah. No. This was not clear to me at all.

I tried with my own strategies but I can only add indicators with /, and then it either integrates with the price chart, or gets onto a separate pane.
How do you set the indicator in the second pane as an external indicator in the strategy of your first pane?

Have you contacted TV support about this?

@mrcrdwd
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried with my own strategies but I can only add indicators with /, and then it either integrates with the price chart, or gets onto a separate pane. How do you set the indicator in the second pane as an external indicator in the strategy of your first pane?

https://www.tradingview.com/blog/en/use-an-input-from-another-indicator-with-your-strategy-19584/

Have you contacted TV support about this?

Yes, already in August. They have confirmed that it's a bug.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for the link. I had all but forgotten about that.
Personally, I haven't seen the need for this as I write Pine myself but I can see how it is useful to apply other people's scripts on more indicators and extend their utility.

I'll add an option to the YAML configuration in order to handle this issue.

@timelyart
Copy link
Owner Author

Choose a reason for hiding this comment

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

Opened issue to handle this bug from Kairos' perspective.

Please sign in to comment.