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

Add a pre-process method #28

Merged
merged 17 commits into from
Sep 30, 2024
Merged

Add a pre-process method #28

merged 17 commits into from
Sep 30, 2024

Conversation

mikecooke77
Copy link
Contributor

@mikecooke77 mikecooke77 commented Sep 23, 2024

Description

This PR addresses the second part of #25 by adding a new method which can be called using yp-preprocessor.

Issue(s) addressed

Resolves #25

Dependencies

No dependencies.

Impact

None

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have run the unit tests before creating the PR

@mikecooke77 mikecooke77 requested a review from orlewis September 23, 2024 15:07
Copy link

@DavidSimonin DavidSimonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made few comments, but I wuld be happy eitherway for most of them.

@DavidSimonin
Copy link

Happy with your comment above.

Regarding the the enviroment variable, should we use something like that to evalusate them instead of a mapping?:

>>> os.path.expandvars('$DATADIR/test/testing')
'/data/users/frsd/test/testing'

@mikecooke77
Copy link
Contributor Author

os.path.expandvars('$DATADIR/test/testing')

I like this as an idea but we are not passing them in as environment variables but as a defined list so I can't see how to get this to work. Unless we want to define it as an environment variable which I would prefer not to do because the define method makes the user do this explicitly.

@matthewrmshin
Copy link
Collaborator

See also yp-data's help on how environment variables are mapped.

  • Environment variables are the default mapping for variable syntax in the file, unless the --no-environment option is specified.
  • Each -D KEY=VALUE overrides.
  • There is also a --no-process-variable option.

@mikecooke77
Copy link
Contributor Author

mikecooke77 commented Sep 27, 2024

Thanks for the comments I have looked at the code on the dataprocessor and used this to update the datapreprocessor so that environment variables can also be used (and turned off).

I don't think for the preprocessor we would ever need to no process variables.

@mikecooke77
Copy link
Contributor Author

@matthewrmshin if you are happy with this code could we get it in the stack I can them start testing the jjdoc / jjaux split which will enable us to get up to date

@matthewrmshin matthewrmshin merged commit eebb5d7 into develop Sep 30, 2024
3 checks passed
@matthewrmshin matthewrmshin deleted the feature/add_preprocess branch September 30, 2024 09:52
@matthewrmshin matthewrmshin added this to the 0.6 milestone Sep 30, 2024
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.

Add a pre-process step
3 participants