Refactoing the NSEPython code

Hey,

Firstly, thank you for the project. It’s helpful.

While going through the code, I see that the project can be written in a better way with a better project structure. I would be happy to help you with it. I can create a new branch & start refactoring it.

Would you be interested in this idea?

2 Likes

It’s an open source project which I made for tools I was in need of . Feel free to do anything you want.

Hi @kashyap

please do take a stab at it… this is very much in the interests of the community. please feel free to put a proposal outline as to what you would like to refactor – outline what are changes that are needed and you are willing to make ammends and seek inputs from the community and also the original author @dexter

thanks

2 Likes

The more unethical diversions in a code, the stronger the community. As said, do whatever you please. But please share here once. If it is better than adipurush movie i will add them in the main documentation with your name :slight_smile:

Hey @dexter & @Nikhil_M Thank you for the feedback.

At the moment, I can recommend the following changes:

  • Use a package manager like either PDM or Poetry
  • Use ruff as linter
  • In place of requirements.txt, have pyproject.toml
  • Move the constants & URLs to a separate module for better readability of the code
  • Extract codes that can be a part of utils
  • I don’t know which version of Python you’re using, but I’d prefer either 3.9 or 3.10
  • Try & use the best python coding practices, like adding type hints & newer syntaxes in-place of existing ones.
  • Add tests, if they make sense

If I have time, I can try to replace Pandas with Polars, but that won’t be a prio for me.

Let me know what you think. If you like the ideas, I can create a branch called code-refactoring & start working on it in my free time.

Hi @Kashyap_Maheshwari

[I am taking a stab at replying first, but hopefully resonates with Amit as well but I know he does care so wait for his reply as well]

The changes that are proposed are definitely looking like are going to break a lot of changes and infact it is mostly to stay ahead with the python community latest trends than what could be considered as stable changes except for couple of things like a) moving the constants and urls to a seperate module for a better readability of the code b) extract codes that can be part of utils c) add the tests as they make sense

these above seem to be low hanging fruits for you to take a stab at without really breaking the existing functionality of the code. they can be multiple iterations as part of the first tranche of changes here.

now, the other changes are more of changing the norms of the current practices which may or may not become the actual standards (like poetry or pyproject.toml have not really become mainstream python ecosystem standards as yet, so I would advise against from proposing them to the community changes just yet, perhaps in the future when it is a widespread ecosystem standard)

once the above first tranche of changes are taken care of, I could suggest to see if it works with python 3.9 or python3.10 (or which ever is more stable python in 3.x series instead of the latest version) as that would help the whole codebase stay inline with the stable python release even in 3.x series, – this could be your second tranche of changes.

regarding the move to polars from pandas, (I was considering it myself a while ago), I would not really advocate it unless there is a really lot of potential in terms of performance improvement that can be measured and also proved.
however, what can be considered or attempted, ofcourse after first tranche and second tranche as priorities, is to consider moving to use pandas 2.0 library – recently @dexter has made all the changes necessary to make the current code base latest pandas 1.x compliant and is now ready for accommodating the move to pandas 2.x without much hassle unless the methods themselves have signficantly changed — but again, all aspects of the code base still need to be thoroughly tested and guarantee the backward functionality

and this is the reason I would suggest add the (missing) tests at first to make sure the functionality and integrity are thoroughly at check all the time.

so to summarize, this would be recommended approach, in my view:

  1. tranche
    o add all the missing functional and integrity tests as they make sense
    o modularize the constants and urls – this improves readability without breaking the tests
    o extract codes that can be part of utils

  2. tranche

    o consider making the codebase work with stable python 3.x series

  3. tranche

    o attempt to make the code base work with stable pandas 2.x series
    o again, make sure all the functional and integrity tests are guaranteeing the backward compatibility

because this module is now deep in almost lot of code bases (thanks to efforts by @dexter ), it is important to make sure any and every codebase change is going to only provide benefits in the interests of the growing community here.

hope this helps – again, I really appreciate your efforts here @Kashyap_Maheshwari

2 Likes

One tiny comment - keep both polar and pandas. Add new functions with _polar maybe.

Always give multiple ways of implementation of a single thing . Like see I kept nsepython vps version and server version separate but with same code base.

Ease of use.

Refactor in a way that old people does not have to change one iota of code as well as new methods also can be accessed. So change the main repo directly with that intention

@dexter & @Nikhil_M I’ve created a PR (Code refactorings by kashyapm94 · Pull Request #28 · aeron7/nsepython · GitHub)

Feel free to take a look at it & give your feedback.

I need feedback on 4 places in the code:

  • Line 135
  • Line 346
  • Line 880
  • Line 999

Taking a closer look at the code, I realized it is hard to refactor it the way I would’ve liked without breaking the older versions. So I’ve used the same function names, same parameter names, same business logic for the moment.

I would highly recommend releasing the changes, if accepted, as a new version & still keep the current version available for installation so that people can choose between the version & if people have issues in my version, then I too can solve the issues.

1 Like

Will take few days. Sitting outside OT as wife has gallbladder removal operation. Then I had to find for good internet as I need to sync roughly 50-60GB