Mention self._vcov in docs

[greenguy33]

@s3alfisc I made the small change we discussed. I'm also interested in continuing to contribute to this project if I can be helpful in any way!

[s3alfisc]

I just realized that things might be a little bit more complicated than just this simple change

Doing as above would introduce a breaking changes as users could no longer run code as

import pyfixest as pf
pf.feols("Y~X1", data = pf.get_data).vcov("hetero").summary()

I'm not really sure if many use such syntax, but I have certainly seen it already =)

One option would be to add a "return_vcov" function argument, which defaults to False, in which case the Feols object would be returned (i.e. the status quo), whereas if True, we'd return the full vcov matrix.

I also wonder if we should add a function that allows users to control if they want to overwrite inferential results in the Feols instance whenever they call .vcov?

I.e. right now, whenever vcov() is called, the _vcov attribute of the Feols instance is updated, and self.get_inference() updates t-statistics, p-values, etc.

What do you think @greenguy33 ?

[s3alfisc]

Another simple option to make the vcov matrix accessible would be to simply make it a "public" attribute, in which case we'd have to rename all references to _vcov to vcov. Maybe that is even the best solution @greenguy33?

[s3alfisc]

I'm also interested in continuing to contribute to this project if I can be helpful in any way!

Super cool, there's plenty of things to do! I'll reach out about this later today =)

[greenguy33]

@s3alfisc This makes sense and I feel like either of the solutions that you proposed would be fine. Maybe just exposing the .vcov attribute of feols is the most intuitive option for now.

Either way, is there a way that a user can find out about how this works, other than looking at this issue or reviewing source code? I feel like documentatupn would be helpful here - unless it already exists and I've missed it, that's something I'd be happy to help work on.

[s3alfisc]

Maybe just exposing the .vcov attribute of feols is the most intuitive option for now.

Then let's go with that solution! The only thing you would have to do is to go to your text editor and replace all of the references to _vcov with vcov 😄

Either way, is there a way that a user can find out about how this works, other than looking at this issue or reviewing source code? I feel like documentatupn would be helpful here - unless it already exists and I've missed it, that's something I'd be happy to help work on.

Maybe we could add this to the inference section of the quickstart notebook?

[greenguy33]

Yes sure! I can get to this soon and make another PR. Sorry for so many questions - but should I be running the tests before sending in new code?

[s3alfisc]

You mean the ci/unit tests? You don't have to run them locally, I can run them via GitHub actions. Just haven't yet done so for the current PR as I think they would fail. Btw, you can also simply update this PR, no need to open a new one =)

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[greenguy33]

Hi @s3alfisc, I updated this pull request by changing to use .vcov instead of ._vcov as the attribute for the feols object, and added a line to the notebook you mentioned explaining the new attribute.
I also searched for other points in the code where the deprecated ._vcov attribute might have been called on the feols object, but did not find any, so I hope that just changing the feols.py file is sufficient to not break anything.
I hope that this is helpful!

[s3alfisc]

Hi @greenguy33 , I made two minor changes. The vcov attribute is always available (users don't need to call vcov(), as this always happens internally). And I also found a self._vcov in the lpdid function. Will merge this after CI passes. Thank you! =)

@all-contributors please add @greenguy33 for code and docs

[allcontributors]

@s3alfisc

I've put up a pull request to add @greenguy33! 🎉

[s3alfisc]

It looks like there are a lot of _vcov calls that we missed throughout the repo ... 😅

[greenguy33]

It looks like there are a lot of _vcov calls that we missed throughout the repo ... 😅

Whoops...I will go back and take a closer look asap

[s3alfisc]

No worries - funnily I also checked and I did not find any other _vcov relics in my first search ... only found them after the CI tests failed 😄

[greenguy33]

Hey @s3alfisc, I'm seeing a lot of errors like this in the failed tests:

FIT.vcov(vcov=vcov_type, data=_data_clean)
TypeError: 'numpy.ndarray' object is not callable

It looks like Python does not support class attributes and methods of the same name. Its default behavior is to try to call the value of the attribute as a method, which fails. I wrote a quick sanity check to demonstrate:

class obj:
    def __init__(self):
        self.test = 1
    def test(self):
        return 2
new_obj = obj()
print(new_obj.test)
> 1
print(new_obj.test())
> TypeError: 'int' object is not callable

I think the Pythonic way to handle this might be to use a property decorator, so that both fit.vcov and fit.vcov(..args..) can be used. However, I also saw some examples where the underscore syntax is used as a more simple solution (so ._vcov, what you had before). Maybe the way it is currently is actually fine. But if you like the property decorator idea I can also implement it that way.

[s3alfisc]

Hm I would think the it's likely best to simply keep things as they are but to prominently point out in the vignette that one can access the vcov matrix via "Feols._vcov"? So basically we would scrap all changes and only keep the notebook updates?

[greenguy33]

Hi @s3alfisc, your plan sounds good to me. I updated the commit to only include the change in the quickstart, and also corrected one other small issue in the quickstart I found ("fixed effects" instead of "fix effects")..
That was a lot of talk for a small change :) but hopefully the documentation will help somebody who is in my position next time.
As I said before I'd like to continue to help contribute to this project, so please let me know how I might be able to help. I will also try to propose some stuff I could work on myself as I continue to use the tool!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

see 28 files with indirect coverage changes

[s3alfisc]

Yes, this was indeed a lot of back and forth for a very small change =) Thank you Hayden (@greenguy33)! Looking forward to continue working with you =) Please suggest all the features that you'd like to see added to pyfixest 😄 Is there any area of stats / econometrics that you're particularly interested in? I'd also be happy to have a chat and introduce you to the codebase / set of open issues =)

[greenguy33]

@s3alfisc I have a software engineering background but am self-taught in statistics. I'm currently working on a PhD dissertation at UC Irvine that involves climate econometric modeling, which is what I needed to use this library for! I think that I will have around 2-3 hours a week to work on this library, as I have been meaning to get more involved with working on open-source codebases and this seems like a great opportunity. A call sometime to discuss the codebase and possible areas to work on sounds great! Maybe we can get in touch via e-mail? Feel free to drop me a line at hfreedma@uci.edu

[s3alfisc]

Sounds awesome! I'll send you an email once I'm back home =)