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

آمار های کلیدی #163

Merged
merged 25 commits into from
May 15, 2022
Merged

آمار های کلیدی #163

merged 25 commits into from
May 15, 2022

Conversation

sfmqrb
Copy link
Collaborator

@sfmqrb sfmqrb commented Apr 19, 2022

#160
توی این pr به ایشوی بالا رسیدگی شده است.
در حال حاضر تمامی آمارهای کلیدی که قابل فهم برای لیست نماد های پکیج در symbols_name.json هستند به صورت یکجا دریافت شده و پروسس می‌شود و در قالب یک خروجی فایل جیسون و یک خروجی دیکشنری با نامگذاری مناسب بر روی فیلدها آماری در اختیار کاربر قرار می‌گیرد.

حقیقت خودم در این فکر بودم که آمارهای کلیدی هر سهم را زیرمجموعه Ticker خودش بیاورم و مخصوص هر سهم به صورت جداگانه فیلدهایی به کلاس Ticker اضافه کنم ولی با توجه به اینکه اگر کسی به قصد فیلترنویسی بخواهد از این آمار ها استفاده کند مطمئنا سریع تر و کم دردسر تر است که یک مجموعه به شکل مستقل دریافت کند از این بابت آن را جداگانه طرح کردم. همچنین ساختار response داده شده مربوط به آمارهای کلیدی در این تصمیم بی تاثیر نبوده است.

این نیز گفتنی است با توجه به ساختار api های بخش دیده‌بان بازار(شامل موارد حقیقی و حقوقی و دیتاهای شناختی هر نماد و همچنین قیمت های تاریخی) که یکجا دیتاها را تحویل می دهد به نظرم حتی خوب است باقی اطلاعاتی را هم که به صورت جداگانه و سهم به سهم از Ticker هر نماد در اختیار کاربر قرار می‌دهیم با یک دستور دیگر به صورت یکجا و کامل نیز در اختیار کاربر قرار دهیم. به نظرم این دو در کنار هم ترکیب کاملی می‌شوند که به طور خاص حالت دوم مناسب فیلترنویسی است و حالت اول مناسب تحلیل یک یا چند سهم به شکل جداگانه است.

در صورت علاقه‌مندی برای اطلاعات بیشتر از قابلیت های دیده‌بان این لینک مناسب است.

هر گونه نکته یا پیشنهادی بود در خدمتم.

@Glyphack
Copy link
Owner

دستت درد نکنه من تا آخر این هفته ریویو رو انجام میدم.

@sfmqrb sfmqrb mentioned this pull request May 3, 2022
@@ -0,0 +1,29 @@
from key_stats import filter_key_value
Copy link
Owner

@Glyphack Glyphack May 3, 2022

Choose a reason for hiding this comment

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

اسم این فایل رو بنظرت بزاریم یه چیزی مثل generate_document.py یا همچین چیزی؟

# بسته: close
# باز: open

class KeyStats(object):
Copy link
Owner

Choose a reason for hiding this comment

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

این ارث بری از object رو تا جایی که میدونم دیگه نیاز نداریم از یه ورژنی به بعد توی پایتون.
یه کلاسی هم پایتون داره به اسم Enum خواستی نگاه بنداز توی همچین جاهایی بدرد میخوره برای ساخت لیست

Copy link
Owner

@Glyphack Glyphack left a comment

Choose a reason for hiding this comment

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

با تشکر. میشه برای این هم فیچر هم تست داشته باشیم؟

return get_index_to_symbol_map(symbol_dic)


def _request_key_stats() -> str:
Copy link
Owner

Choose a reason for hiding this comment

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

فکر کنم این تابع بدون اینکه ریسپانسش داده بشه به _get_list_of_processed_stats استفاده نشه جایی درسته؟ ادغامشون کنیم با هم که خروجی این تابع واضح تر باشه که چی هست؟ برای اینکه خروجی یکم واضح تر باشه چه دیتایی توشه(مثلا من نمیدونم چی داخلشه چون اصلا این API رو ندیدم)‌اگه بشه dataclass بسازیم و فیلدهارو مشخص کنیم خیلی عالی میشه.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

به نظرم با توجه به خروجی تابع خیلی نمی تونیم از توش دیتاکلاسی در بیاریم ولی ایده خوبیه که توابعی که گفتی رو با هم ترکیب کنیم. یه مقداری زیادی شاید سعی کردم این تابع try catch بنویسم. واقعا هم ایده ای ندارم چه قدر به درد بخوره.

Copy link
Owner

Choose a reason for hiding this comment

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

ایده‌ای که داری خوبه ولی خب کاری که این تابع میکنه اینه که ورودی رو میگیره و پاس میده به یه تابع دیگه سر همین میتونیم همون تابع رو صدا کنیم خودمون و ارور رو هم هندل کنیم که یه level کمتر فانکشن کال داشته باشیم.

return idxs, values


def get_aggregated_key_stats(base_path=None, to_json=False)\
Copy link
Owner

Choose a reason for hiding this comment

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

پوشه دیتا رو من تو نظرم بود که کلا چیزای ثابت رو بزاریم که هر چند وقت آپدیت میشه. این تابع رو کاربر میخواد استفاده کنه برای گرفتن اطللاعات بنظرم ببریمش بیرون یه جایی کنار تیکر ودانلود

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ایده ی خوبیه. حتما.

@sfmqrb
Copy link
Collaborator Author

sfmqrb commented May 3, 2022

یه سوال کلی هم داشتم. یه کم به نظرم می رسه خروجی دیکشنری ای که متد get_aggregated_key_stats و در صورت سیو در قالب json داره میده با روند پکیج که pandas dataframe کار میکنه در تضاده.
یه کم ملاحظه مموری رو داشتم که این کار رو کردم چون برای خیلی از نماد ها بخشی (بخش بزرگی حتی) از فیلد ها خالیه.
ولی به نظرم تهش بهتره یه خروجی دیتافریم بدم به جای دیکشنری و جیسون و به جای کلیدهای خالی هم مثلا None بذارم. نظرت چیه؟

@Glyphack
Copy link
Owner

Glyphack commented May 5, 2022

یه سوال کلی هم داشتم. یه کم به نظرم می رسه خروجی دیکشنری ای که متد get_aggregated_key_stats و در صورت سیو در قالب json داره میده با روند پکیج که pandas dataframe کار میکنه در تضاده. یه کم ملاحظه مموری رو داشتم که این کار رو کردم چون برای خیلی از نماد ها بخشی (بخش بزرگی حتی) از فیلد ها خالیه. ولی به نظرم تهش بهتره یه خروجی دیتافریم بدم به جای دیکشنری و جیسون و به جای کلیدهای خالی هم مثلا None بذارم. نظرت چیه؟

آره موافقم. کلا چیزایی که دیتافریم هم نیست خیلی سوال پرسیده میشه که چطوری باید استفاده بشه بنظرم این هم دیتافریم باشه بهتره.

@sfmqrb sfmqrb requested a review from Glyphack May 13, 2022 10:32
@Glyphack Glyphack merged commit 9b1d9f4 into Glyphack:master May 15, 2022
@sfmqrb sfmqrb deleted the key_stats branch May 15, 2022 16:41
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.

2 participants