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

Rename and reorganise files #746

Closed
7 tasks done
Tracked by #749
nikosbosse opened this issue Mar 24, 2024 · 8 comments · Fixed by #940
Closed
7 tasks done
Tracked by #749

Rename and reorganise files #746

nikosbosse opened this issue Mar 24, 2024 · 8 comments · Fixed by #940

Comments

@nikosbosse
Copy link
Contributor

nikosbosse commented Mar 24, 2024

  • convencience-functions.R should be renamed to transform_forecasts.R
  • set_forecast_unit() should be moved to a file get_forecast_unit.R (maybe forecast_unit.R?) together with get_forecast_unit().
  • correlations.R should be get_correlations.R
  • different get_-functions should be moved out of get_-functions.R
  • metrics-range.R --> metrics-interval_range.R
  • customise_metric() should maybe get its own file. Alternatively, the corresponding tests should be moved.
  • the current function validate_scores should really be assert_scores()
@seabbs
Copy link
Contributor

seabbs commented Apr 12, 2024

Is this all done now?

@nikosbosse
Copy link
Contributor Author

We did another set of renaming/restructuring files previously, this one is still open.

@sbfnk
Copy link
Contributor

sbfnk commented Apr 19, 2024

get_-functions.R also contains some assert_... and test_... functions which I think should be moved to one of the check-....R fiiles.

@jamesmbaazam
Copy link
Contributor

jamesmbaazam commented Apr 19, 2024

@nikosbosse You could perhaps do this as a final step before release since it's not a functional change?

@nikosbosse
Copy link
Contributor Author

Following one of Sam's suggestions, we should think about moving all the files related to one forecast class to one file (i.e. everything related to converting into a forecast and assertions and tests and checks etc.)

@seabbs
Copy link
Contributor

seabbs commented Aug 6, 2024

I really like this from the view of thinking about implementing new classes as the process to do so is then copy a single file that you know contains everything you need to create and then work through doing so. I can see the argument that it might be hard to get an impression of the overall package structure though

@jamesmbaazam
Copy link
Contributor

Following one of Sam's suggestions, we should think about moving all the files related to one forecast class to one file (i.e. everything related to converting into a forecast and assertions and tests and checks etc.)

Agreed. It makes it easier to find everything about the class in one place. See dist_spec.R for example.

@jamesmbaazam
Copy link
Contributor

I really like this from the view of thinking about implementing new classes as the process to do so is then copy a single file that you know contains everything you need to create and then work through doing so. I can see the argument that it might be hard to get an impression of the overall package structure though

That's how I learnt to develop classes actually.

@nikosbosse nikosbosse mentioned this issue Oct 3, 2024
9 tasks
seabbs pushed a commit that referenced this issue Oct 7, 2024
* reorganise some files

* rename files

* more moving files

* more moving functions

* move a lot of files

* juggle more things around

* update docs

* appease the gods of linting

* move tests around

* lint for the lint god

* keywords for the keyword got
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants