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

Bug in macrodata/_dwn_idbank_files #231

Open
tgrandje opened this issue Jan 17, 2025 · 2 comments
Open

Bug in macrodata/_dwn_idbank_files #231

tgrandje opened this issue Jan 17, 2025 · 2 comments

Comments

@tgrandje
Copy link
Collaborator

tgrandje commented Jan 17, 2025

There is something strange going on on those lines.

  1. should it be retrieved on line 44, data is never used (as files is never empty) ;
  2. anyway, file_to_dwn set as constant here is not available as of today
  3. further more, _dwn_idbank_file needs a session argument, so this is always trigerring the exception.

@hadrilec can you detail what the expected behaviour should be ?

I'm not really sure it this sample should be dropped altogether or if there is a reason still to keep it...

@tgrandje tgrandje changed the title Bug macrodata/._dwn_idbank_files Bug in macrodata/._dwn_idbank_files Jan 17, 2025
@tgrandje tgrandje changed the title Bug in macrodata/._dwn_idbank_files Bug in macrodata/_dwn_idbank_files Jan 17, 2025
@hadrilec
Copy link
Contributor

hadrilec commented Jan 20, 2025

hi, the intention was to allow the user the ability to change manually the file retrieved from insee.fr. This list is updated manually on insee side, so it is not unlikely that the file changes name. These lines should then be considered as a backup before a permanent fix is released in a new version of the package. The code first try to use the file the user intentionally sets and then uses dates to create potential correct urls. The quality of this snippet of code could of course be better. Shall I provide you with more details?

@tgrandje
Copy link
Collaborator Author

OK, so the missing part is the session argument to _dwn_idbank_file. I'll fix that in the PR fixing the github actions timeouts (makes sense as this is already messing with the requests sessions).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants