search

py-econometrics / pyfixest

Fast High-Dimensional Fixed Effects Regression in Python following fixest-syntax
https://py-econometrics.github.io/pyfixest/pyfixest.html
MIT License
117 stars 27 forks source link

Added solver to feols and created new test file for it #513

Open saidamir opened 1 week ago

saidamir commented 1 week ago

Hi I added a solver to feols class and created a test file for it. Once approved, I will add the solver to other classes as well, as I want to complete one class at this stage.

saidamir commented 6 days ago

@s3alfisc would you please review my pr at your free time? Thanks!

s3alfisc commented 6 days ago

Hi @saidamir, sorry, I did not understand that this was already ready for review. This looks good, thank you! =) What next step do you have in mind? Will you add solver arguments to Fepois and Feiv or add arguments for the feols() and fepois() APIs?

saidamir commented 5 days ago

The test is failing per logs, as I can see, and the error: "FAILED tests/test_solver.py::test_solver_equivalence - TypeError: Y must be a numpy array." I could not figure out why this was happening, it looks to me that this bug is not cause by my changes but it was in the original design of the get_data() method and feols class. The output of get_data() is not compatible with Feols methods. What do you think?

s3alfisc commented 12 hours ago

Hi Aziz (@saidamir) - if you move the solve_ols method to Feols and delete it in the Feiv and Fepois classes, then this looks good to merge (pending CI tests not failing, which I don't expect).

@all-contributors please add @saidamir for code.

allcontributors[bot] commented 12 hours ago

@s3alfisc

I've put up a pull request to add @saidamir! :tada: