Redo Screener

[R5dan]

It is annoying that the types are wrong on Screener, I find myself looking at the source code to determine the types of the Screener class so this is a fix

[ericpien]

1. what do you mean the "types are wrong"?
2. What does it mean to "determine the types of the Screener class"?
3. How can we improve the documentation for Screener to avoid "looking at the source code"?

[R5dan]

1. what do you mean the "types are wrong"?

The return types on the Screener are wrong (for example dict not dict_keys on predefined_bodies which seems trivial but the methods are different on them) etc

2. What does it mean to "determine the types of the Screener class"?

I go into the source code to work out what types different properties are as when I run code (which would be correct with the types) it fails (due to incorrect type)

I also have to go looking for what string literals are allowed in set_predefined_body when they are hard coded in and most type checkers can prompt you to which ones to use.

3. How can we improve the documentation for Screener to avoid "looking at the source code"?

Mostly fixed it with this PR. Take a look if you want.

I also made the the types run faster and be more runtime safe: by putting the type in a string it only has to construct a string and not the full class and it also means it's easier to fix circular import errors or the class not actually existing for some reason as some of them could be moved under t.TYPE_CHECKING if needed

[R5dan]

@ValueRaider What do you think?

[ValueRaider]

How can we improve the documentation for Screener

Improve how user sets up a screener query with a simpler intermediate form:

from dataclasses import dataclass
from typing import List, Union, Any

@dataclass
class Query:
    operator: str
    operands: List[Union['Query', Any]]

    def to_dict(self) -> dict:
        return {
            "operator": self.operator,
            "operands": [op.to_dict() if isinstance(op, Query) else op 
                         for op in self.operands]}

def value_in(key: str, *values):
   return Query("or", [Query("eq", [key, v]) for v in values])

query = Query(
    "and",
    [
        value_in("exchange", "NMS", "NYQ"),
        Query("lt", ["epsgrowth.lasttwelvemonths", 15])
    ]
)

Should shorten the screener html page by a lot.

[R5dan]

Should I also add #154 into this one?

[ValueRaider]

ok

[R5dan]

@ValueRaider any other predefined keys you know off? Still don't know where you found that 52 week gain

[R5dan]

Also why is Query and EquityQuery different? EquityQuery doesn't even have region etc.

[ValueRaider]

@ValueRaider any other predefined keys you know off? Still don't know where you found that 52 week gain

I just navigated the gainers webpage with Firefox network inspector open.

[R5dan]

@ValueRaider Still need to add proper logging (not just print statements) and docs etc, but what do you think of this.
I am getting the error:

{
    "finance":
        {
            "result":null,
            "error":
                {
                    "code":"internal-error",
                    "description":"sort field is not a field: fundnetassets"
                }
         }
}

returned by yahoo. I don't understand why as this part is the same as the original and that works fine. Any ideas?

[ericpien]

doesn't this result in unintentional updates of certain fields?

i.e. user with body offset = 50, goes to patch the sortField but offset will reset to 0.

[R5dan]

Yes it would, except it is not implemented yet. Will change it when I do implement it

[ericpien]

why do we need a separate class for this? Isn't this just a Screener with pre-defined fields?

[R5dan]

No, it sends a different request to a different endpoint with a different response

[ericpien]

If you access the yahoo's screener via: https://finance.yahoo.com/research-hub/screener, the request is made to the same endpoint for the pre-defined queries.

imho, it's better to not create a separate class for this to allow user to construct a Screener that reflects the pre-defined body, and edit it as they need for flexibility. i.e. offset, or exchange requirements.

[R5dan]

Sure but:

AttributeError: module 'yfinance' has no attribute 'screener'

Weird, I don't get that error:

from yfinance.screener import Screener

predefined = Screener.predefined("day_gainers", count=25)

[ericpien]

Below are just my two cents:

I think this affects more than just types as initially proposed and there isn't much example/test code to infer. Can you edit the tests for the changes to assist in code review?

I'm actually not sure what problem this PR is trying to solve anymore.

Also why is Query and EquityQuery different? EquityQuery doesn't even have region etc.

This was done so future developers can extend to different types of queries but leverage the same screener class. i.e. quoteType = "MUTUALFUND"

I also made the the types run faster and be more runtime safe: by putting the type in a string it only has to construct a string and not the full class and it also means it's easier to fix circular import errors or the class not actually existing for some reason as some of them could be moved under t.TYPE_CHECKING if needed

I personally have a preference for using the typing module instead for readability and maintainability.

[R5dan]

It was initially just to edit the Types but then I was asked to fix the Query and EntityQuery and make screener better and easier. I understand what you are saying about having no direction, it is now to redo the screener to make it easier to use and make custom queries etc.

I personally have a preference for using the typing module instead for readability and maintainability.

Personally I generally prefer as little from typing as possible (except certain things)

there isn't much example/test code to infer

Yes, I am going to add more, just wanted to see whether people like this, or want specific and overall changes before implementing more specific, adding docs and tests, I just wanted to get something working. This is a draft pr so far.

[ValueRaider]

Ok, I have your code running now. I've deleted those comments to tidy this thread and get back on track.

I'm actually not sure what problem this PR is trying to solve anymore.

Might be my fault. I only just now understand that @ericpien implemented a Query class that appears to do what I wanted. I need to spend more time with the existing screener and query, simplify the docs.

[R5dan]

Might be my fault. I only just now understand that @ericpien implemented a Query class that appears to do what I wanted.

Do you want me to stop this then?

[ValueRaider]

Do you want me to stop this then?

Pause for now. I'll look more over weekend.

[R5dan]

@ValueRaider Made a decision? Are you just going to go with #2207

[ValueRaider]

@R5dan relax, let's work through #2207 then after see where this PR fits in.

[R5dan]

I was just checking. Wanted to see if you wanted me to continue or not, so Ill pause for now.