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

[Refactor]: purge deprecated configuration option data_path #472

Open
pearsonca opened this issue Jan 22, 2025 · 7 comments · May be fixed by #480
Open

[Refactor]: purge deprecated configuration option data_path #472

pearsonca opened this issue Jan 22, 2025 · 7 comments · May be fixed by #480
Assignees
Labels
config Relating to configuration files or their framework. docstring Relating to in-code documentation. documentation Relating to ReadMEs / gitbook / vignettes / etc. high priority High priority.

Comments

@pearsonca
Copy link
Contributor

The data_path configuration option is no longer supported and has not been for some time.

As such, we should now purge all specific deprecation-related code (e.g. warnings specifically about the presence of this argument), fix all documentation to remove it, and ensure vestigal code concerning it goes away.

@pearsonca pearsonca assigned pearsonca and emprzy and unassigned pearsonca Jan 22, 2025
@pearsonca pearsonca added documentation Relating to ReadMEs / gitbook / vignettes / etc. config Relating to configuration files or their framework. docstring Relating to in-code documentation. labels Jan 22, 2025
@pearsonca
Copy link
Contributor Author

pearsonca commented Jan 22, 2025

This could be accomplished in stages:

  • remove references in gitbook
  • remove warnings in gempyor
  • remove references in scripts
  • remove references in r packages

Probably gitbook items first (@TimothyWillard hotfix? since its already deprecated), but then the rest in any order (probably non-hotfix).

@emprzy
Copy link
Collaborator

emprzy commented Jan 22, 2025

Quick question for anyone to answer:

Image

Not sure what weight data_var has, as it as listed as a column in the data_path file. How should I handle data_var?. Should I remove it as well, since data_path will not longer exist? Or should I re-route its explanation to a different file?

ss from this page on the gitbook.

@pearsonca
Copy link
Contributor Author

Hmm - I think those are explicitly labelled "OLD" at the moment, so we can ignore them for now.

At some point we'll want to support flepimop-versioned-user guide that people can select the version they're working from. I think sphinx may "just work" to provide that kind of support, if we approach the general user guide as part of the API.

That's a longer term effort, I'd guess.

@emprzy
Copy link
Collaborator

emprzy commented Jan 23, 2025

Hmm - I think those are explicitly labelled "OLD" at the moment, so we can ignore them for now.

At some point we'll want to support flepimop-versioned-user guide that people can select the version they're working from. I think sphinx may "just work" to provide that kind of support, if we approach the general user guide as part of the API.

That's a longer term effort, I'd guess.

Yep you're right, those gitbook files are labeled OLD. But this same data_var as a column in data_path also appears in inline roxygen2 documentation. So, I guess my question still stands, as how would you prefer this reference to be resolved? Photo provided of R documentation:

Image

@pearsonca
Copy link
Contributor Author

which R is that from? the scripts or one of the packages?

If those params are still used in the corresponding function args, that param documentation can remain until we also revise those function signatures (in later stages of resolving this deprecation issue).

@emprzy
Copy link
Collaborator

emprzy commented Jan 23, 2025

it's from yaml_utils.R

It's in the flepiconfig R package. data_var seems pretty embedded in the functions of that file.

@pearsonca
Copy link
Contributor Author

Got it - I'm actively working on the refactor for it, so don't worry about anything in flepiconfig.

@emprzy emprzy linked a pull request Jan 23, 2025 that will close this issue
@TimothyWillard TimothyWillard linked a pull request Jan 23, 2025 that will close this issue
@TimothyWillard TimothyWillard added the high priority High priority. label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config Relating to configuration files or their framework. docstring Relating to in-code documentation. documentation Relating to ReadMEs / gitbook / vignettes / etc. high priority High priority.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants