Skip to content

SilNlpEnv refactoring#1062

Open
benjaminking wants to merge 4 commits intomasterfrom
environment_refactoring
Open

SilNlpEnv refactoring#1062
benjaminking wants to merge 4 commits intomasterfrom
environment_refactoring

Conversation

@benjaminking
Copy link
Copy Markdown
Collaborator

@benjaminking benjaminking commented May 8, 2026

This PR includes two major changes to SilNlpEnv:

  1. It is now immutable, with factory methods to initialize certain paths to custom values
  2. It is no longer used as a global variable, but is instead injected (passed) into each context where it is used

I should mention that this refactoring is far from the final word on how the environment should be treated. It's a step in the right direction, but in most places where SilNlpEnv is used, there is further refactoring to do. It is now frequently an argument passed to various functions, and in almost all of these cases, it would be more appropriate to wrap these functions in classes, so that long-lived context information, like the environment, can be provided during object creation, and the request-scoped information can remain as the method parameters. Furthermore, many places in the code should probably not be interacting directly with the environment object, but with a higher-level abstraction (something like ExperimentFileManager or ParatextProject).

@TaperChipmunk32, can you take a close look at the onboarding scripts? I had to make several changes to those scripts, but I don't know the right way to test them, so those changes have not been tested.


This change is Reviewable

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

Successfully merging this pull request may close these issues.

1 participant