Adding literals to feols and fepois api's

[marcandre259]

Reference to #671. Pretty literal interpretation. Added a small validate_literal_argument function, taken from the predict method of Feols (assumption is it may be used elsewhere also).

[marcandre259]

pre-commit.ci autofix

[codecov]

Codecov Report

Attention: Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pyfixest/estimation/literals.py	83.33%	2 Missing ⚠️

Flag	Coverage Δ
core-tests	77.73% <90.47%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pyfixest/did/estimation.py	96.55% <100.00%> (+0.06%)	⬆️
pyfixest/did/lpdid.py	94.62% <100.00%> (+0.05%)	⬆️
pyfixest/estimation/__init__.py	100.00% <100.00%> (ø)
pyfixest/estimation/estimation.py	95.91% <100.00%> (+0.04%)	⬆️
pyfixest/estimation/feols_.py	83.06% <100.00%> (+0.07%)	⬆️
pyfixest/estimation/feols_compressed_.py	83.43% <100.00%> (ø)
pyfixest/estimation/fepois_.py	87.20% <100.00%> (-1.90%)	⬇️
pyfixest/estimation/literals.py	83.33% <83.33%> (ø)

... and 1 file with indirect coverage changes

[s3alfisc]

Looks very good to me. Will leave this open until tomorrow to see if @leostimpfle has some thoughs, and then merge after we get his feedback. Thanks @marcandre259!

[leostimpfle]

Thanks @marcandre259 ! Looks good to me overall, just have a few comments

[marcandre259]

pre-commit.ci autofix

[s3alfisc]

pre-commit.ci autofix

[s3alfisc]

It's a little bit annoying, but whenever the autofix corrects code, one has to run it again to pass the checks (because the previous did not pass as it triggered the fix) @marcandre259

[s3alfisc]

Looks like mypy is complaining about the use of the _LiteralGenericAlias:

pyfixest/estimation/literals.py:1: error: Module "typing" has no attribute "_LiteralGenericAlias"  [attr-defined]

Maybe we can simply check for a literal type via smth like

    if get_origin(literal) is not Literal:
        raise TypeError("Expected a Literal type as the second argument")

?

I would not overthink this as it's in the end behavior we would (hopefully) catch in a review process?

[s3alfisc]

Another thing I oversaw in my initial review - feols and fepois check input types in

pyfixest/pyfixest/estimation/estimation.py

Line 617 in a3ed27a

def _estimation_input_checks(

Maybe we could update them to use the Literal validation function whenever relevant?

[marcandre259]

Looks like mypy is complaining about the use of the _LiteralGenericAlias:

pyfixest/estimation/literals.py:1: error: Module "typing" has no attribute "_LiteralGenericAlias"  [attr-defined]

Maybe we can simply check for a literal type via smth like

    if get_origin(literal) is not Literal:
        raise TypeError("Expected a Literal type as the second argument")

?

I would not overthink this as it's in the end behavior we would (hopefully) catch in a review process?

Hi @s3alfisc,

Indeed, I'll modify it with a similar check. I did notice that Literal was not really a type without being specific Literal["..."], at least when trying out type(literal) is Literal. But maybe it works with get_origin then?

[marcandre259]

pre-commit.ci autofix

[s3alfisc]

Thanks @marcandre259 ! This looks good to me know =)

[s3alfisc]

Just fyi @marcandre259 , the test failures in the main branch stem back from changes to the actions wfl I merged yesterday and are completely unrelated to this PR =)