Bknight regex migration #551
Conversation
b-knight
force-pushed
the
bknight_regex_migration
branch
from
5380439 to
1b25bba
Compare
There was a problem hiding this comment.
Happy to merge after the unit tests run succesfully!
@all-contributors please add @b-knight for code.
|
|
||
| variable = c_match.group(1) | ||
| level = t_match.group(1) | ||
| return 'C(' + variable + ')', level[0 : level.rfind("]")] |
There was a problem hiding this comment.
Maybe we want to drop the C(...) wrapper here? Then we could also drop it further below?
There was a problem hiding this comment.
Actually, let's do this in a separate PR, as this clashes with Apoorva's PR #553. Likely easier to merge both as is and then adjust =)
There was a problem hiding this comment.
On this part: level[0 : level.rfind("]")] - did you encountered a string where it was needed because the regex did not handle it by itself, or is this just to be on the safe side?
There was a problem hiding this comment.
The regex matching was leading to values like 'f3' whereas the unit test was expecting 'C(f3)' so this wrapper is necessary to clear the unit test. Similarly, the initial matching was outputting values such as "['ios'" and "['ios', 'android'" with the rightmost bracket missing.
Codecov ReportAttention: Patch coverage is
|
|
@all-contributors please add @b-knight for code |
|
This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected string in JSON at position 3438 |
|
@all-contributors please add @b-knight for code |
|
I've put up a pull request to add @b-knight! 🎉 |
Extension of initial PR.
This pull request primarily involves changes to the
pyfixestpackage, specifically thefeols_.pyanddev_utils.pyfiles. The changes are aimed at improving code quality and readability, as well as introducing a new utility function to extract variable levels.Code quality and readability improvements:
pyfixest/estimation/feols_.py: Removed the import statement for theremodule as it is no longer used directly in this file.pyfixest/estimation/feols_.py: Reformatted theValueErrormessages in theritestandplot_ritestmethods to improve readability. [1] [2]New utility function and its usage:
pyfixest/utils/dev_utils.py: Added a new utility function_extract_variable_levelto extract variable and level from a given string. This function uses regular expressions to match patterns and extract the required information. If the patterns are not found, aValueErroris raised.pyfixest/estimation/feols_.py: Replaced the regular expression matching and extraction logic in thefixefmethod with a call to the new_extract_variable_levelfunction. This change simplifies the code and improves readability.tests/test_predict_resid_fixef.py: Added a new test casetest_extract_variable_levelto verify the correct extraction of lists, floats, and integers by the_extract_variable_levelfunction.Other changes:
pyfixest/estimation/feols_.py: Added the_extract_variable_levelfunction to the import statements frompyfixest.utils.dev_utils.tests/test_predict_resid_fixef.py: Imported the_extract_variable_levelfunction for use in the test case.