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

Intraday f index #165

Merged
merged 10 commits into from
May 15, 2022
Merged

Intraday f index #165

merged 10 commits into from
May 15, 2022

Conversation

sfmqrb
Copy link
Collaborator

@sfmqrb sfmqrb commented May 2, 2022

#164
به ایشو بالا رسیدگی شد.
همینطور برای شاخص مالی یک کلاس به نام FinancialIndex تعریف شد تا داده های دیگر مربوط به شاخص ها نیز از آن دریافت شود.
البته می توانستم از http://uat.tsetmc.com/IndexInfo/{financial_index} مثل http://uat.tsetmc.com/IndexInfo/30106839080444358 استفاده کنم ولی چند باری امتحان کردم و به نظرم api قابل اعتمادی نیامد.

@lgtm-com
Copy link

lgtm-com bot commented May 2, 2022

This pull request introduces 5 alerts when merging 16b8d30 into d8070ed - view on LGTM.com

new alerts:

  • 5 for Variable defined multiple times

return df

@property
def _financial_index_page_soup(self):
Copy link
Owner

Choose a reason for hiding this comment

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

خیلی کار خوبی کردی که این متدهای پارس کردن صفحه رو جدا غیر پابلیک کردی و جدا هست. من خودم ترجیحم این هست که یه فایل دیگه به اسم مثلا parser داشته باشیم کلا متودهای پارس کردن دیتا از html توی اون باشه مثل همونی که توی ticker هست. ولی این خودش قدم خوبیه برای اول کار که پیش بریم.

from pytse_client.download import download_financial_indexes


class FinancialIndex:
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
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.

خیلی ممنون همه چیز عالیه تست بنویسیم برای کلاس جدید و مرج کنیم.

@sfmqrb
Copy link
Collaborator Author

sfmqrb commented May 3, 2022

سلام @Glyphack
ممنونم از ریویوت
حتما تست ها رو درست میکنم و می فرستم یه چند روز دیگه.
خیلی خوب میشه وقت کردی بقیه pr ها رو هم یه ریویو بکنی چون مواردی هستن که اگه زودتر مرج بشن کار روی پکیج سریع تر میشه.
الان مثلا این ایشویی که برای باگ باز کردم #166 باید برای حلش توی ticker دست برد که مثلا #162 باهاش مرتبطه و بهتره تا مرج نشده کاریش نکنم.
اون بخش دیتاهای فیلترنویسی هم یه بخشی از داده هاش مونده که بگیریم البته مهماش رو گرفتیم ولی اگه زودتر ریویو بشه #163 کمک میکنه به سرعت کار منم.

@sfmqrb sfmqrb requested a review from Glyphack May 12, 2022 22:00
@Glyphack Glyphack merged commit a9c6601 into Glyphack:master May 15, 2022
@sfmqrb sfmqrb deleted the intraday-fIndex 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