Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating spreg to 1.7 #158

Merged
merged 13 commits into from
Sep 24, 2024
Merged

Updating spreg to 1.7 #158

merged 13 commits into from
Sep 24, 2024

Conversation

pedrovma
Copy link
Member

Changelog:

  • Add the function NSLX to allow for estimating non-linear SLX models.
  • Allow Kernel weights in SLX models
  • Add observation-specific multiplier effects
  • Other enhancements and bug fixes

Changelog:

-              Add the function NSLX to allow for estimating non-linear SLX models.
-              Allow Kernel weights in SLX models
-              Add observation-specific multiplier effects
-              Other enhancements and bug fixes
@knaaptime
Copy link
Member

wow, sweet!

have been meaning to catch up because I've been using spreg lots lately. Been meaning to open another issue, but one thing I've noticed lately is the slx_vars argument doesn't seem to be working properly (at least in GM_Lag)... if I pass a list of bools as requested, it still gives back all WX, not just the specified X.

Didnt want to raise before investigating more, but while I have you here.... :P

@jGaboardi jGaboardi added bug Something isn't working enhancement New feature or request labels Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 63.86010% with 279 lines in your changes missing coverage. Please review.

Project coverage is 69.3%. Comparing base (f50f0b1) to head (f498b51).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
spreg/sputils.py 20.3% 59 Missing ⚠️
spreg/nslx.py 74.0% 52 Missing ⚠️
spreg/utils.py 56.7% 26 Missing ⚠️
spreg/error_sp_het.py 45.2% 23 Missing ⚠️
spreg/user_output.py 61.0% 23 Missing ⚠️
spreg/ols_regimes.py 35.5% 20 Missing ⚠️
spreg/output.py 81.1% 17 Missing ⚠️
spreg/twosls_sp_regimes.py 62.5% 15 Missing ⚠️
spreg/ml_lag_regimes.py 64.3% 10 Missing ⚠️
spreg/ml_lag.py 58.8% 7 Missing ⚠️
... and 6 more
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #158     +/-   ##
=======================================
- Coverage   69.5%   69.3%   -0.2%     
=======================================
  Files         42      43      +1     
  Lines      10059   10426    +367     
=======================================
+ Hits        6988    7221    +233     
- Misses      3071    3205    +134     
Files with missing lines Coverage Δ
spreg/__init__.py 100.0% <100.0%> (ø)
spreg/diagnostics_sp.py 98.3% <100.0%> (ø)
spreg/error_sp_hom.py 83.2% <ø> (ø)
spreg/ols.py 73.9% <78.6%> (+2.0%) ⬆️
spreg/twosls_regimes.py 65.4% <81.2%> (-0.2%) ⬇️
spreg/twosls_sp.py 71.8% <85.7%> (-0.3%) ⬇️
spreg/twosls.py 76.8% <73.7%> (-2.1%) ⬇️
spreg/diagnostics.py 91.2% <64.7%> (+1.3%) ⬆️
spreg/ml_lag.py 82.3% <58.8%> (ø)
spreg/skater_reg.py 11.0% <0.0%> (ø)
... and 9 more

... and 1 file with indirect coverage changes

@pedrovma
Copy link
Member Author

@knaaptime , @jGaboardi , should we stop testing in python 3.9 since it can't solve the environment using the latest version of libpysal?

@pedrovma
Copy link
Member Author

wow, sweet!

have been meaning to catch up because I've been using spreg lots lately. Been meaning to open another issue, but one thing I've noticed lately is the slx_vars argument doesn't seem to be working properly (at least in GM_Lag)... if I pass a list of bools as requested, it still gives back all WX, not just the specified X.

Didnt want to raise before investigating more, but while I have you here.... :P

Could you please check again using this code version to see if it is resolved?

@knaaptime
Copy link
Member

will do

@knaaptime
Copy link
Member

should we stop testing in python 3.9 since it can't solve the environment using the latest version of libpysal?

yes, we test on the last 3 released versions of python (so 3.10-3.12 right now)

@knaaptime
Copy link
Member

fwiw, i get a big warning on import now with this version

import spreg
/Users/knaaptime/Dropbox/projects/spreg/spreg/utils.py:224: SyntaxWarning: invalid escape sequence '\{'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/utils.py:263: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/utils.py:635: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/output.py:138: SyntaxWarning: invalid escape sequence '\_'
  df_2['Variable'] = df_2['Variable'].str.replace("_", "\_").str.replace("%", "\%")
/Users/knaaptime/Dropbox/projects/spreg/spreg/output.py:138: SyntaxWarning: invalid escape sequence '\%'
  df_2['Variable'] = df_2['Variable'].str.replace("_", "\_").str.replace("%", "\%")
/Users/knaaptime/Dropbox/projects/spreg/spreg/diagnostics_sp.py:291: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/diagnostics_sp.py:444: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/diagnostics_sp.py:922: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/error_sp_het.py:1575: SyntaxWarning: invalid escape sequence '\l'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/error_sp_hom.py:1546: SyntaxWarning: invalid escape sequence '\p'
  """

@lanselin
Copy link
Member

fwiw, i get a big warning on import now with this version

import spreg
/Users/knaaptime/Dropbox/projects/spreg/spreg/utils.py:224: SyntaxWarning: invalid escape sequence '\{'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/utils.py:263: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/utils.py:635: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/output.py:138: SyntaxWarning: invalid escape sequence '\_'
  df_2['Variable'] = df_2['Variable'].str.replace("_", "\_").str.replace("%", "\%")
/Users/knaaptime/Dropbox/projects/spreg/spreg/output.py:138: SyntaxWarning: invalid escape sequence '\%'
  df_2['Variable'] = df_2['Variable'].str.replace("_", "\_").str.replace("%", "\%")
/Users/knaaptime/Dropbox/projects/spreg/spreg/diagnostics_sp.py:291: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/diagnostics_sp.py:444: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/diagnostics_sp.py:922: SyntaxWarning: invalid escape sequence '\d'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/error_sp_het.py:1575: SyntaxWarning: invalid escape sequence '\l'
  """
/Users/knaaptime/Dropbox/projects/spreg/spreg/error_sp_hom.py:1546: SyntaxWarning: invalid escape sequence '\p'
  """

this seems to be something new in 3.12, a different way in which escape sequences are formatted, see below:

from stack overflow

In Python 3.12+ the error message is changed from a DeprecationWarning to a SyntaxWarning

You can't just go putting backslashes in string literals whenever you want one. A backslash is only allowed when part of one of the valid escape sequences, and it will cause a DeprecationWarning (< 3.12) or a SyntaxWarning (3.12+) otherwise. For example \A isn't a valid escape sequence:

So you should always use raw strings or \.

I always turn warnings off :-)

@lanselin
Copy link
Member

that should have been

So you should always use raw strings or \.

@lanselin
Copy link
Member

for some reason it takes out the second backslash, so it should be backslash backslash

@jGaboardi
Copy link
Member

for some reason it takes out the second backslash, so it should be backslash backslash

In markdown you can escape each backslash in regular text like --> "this is 4 backslashes but renders as two: \\" or you can put it in a code formatted block like --> \\

@lanselin
Copy link
Member

lanselin commented Sep 20, 2024 via email

@pedrovma
Copy link
Member Author

@knaaptime, I believe we are all set from our side! Many thanks.

@knaaptime knaaptime merged commit 8c19228 into pysal:main Sep 24, 2024
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants