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 option to just wing it with encoding #186

Closed
MicahGale opened this issue Nov 16, 2023 · 5 comments · Fixed by #333
Closed

Add option to just wing it with encoding #186

MicahGale opened this issue Nov 16, 2023 · 5 comments · Fixed by #333
Assignees
Labels
feature request An issue that improves the user interface. good first issue Good for newcomers
Milestone

Comments

@MicahGale
Copy link
Collaborator

In #118 we changed over to only using ASCII encoding. Though in the wild many problems use other encoding schemes. We should either add an option to not use ASCII, which defaults to UTF-8, or to allow the user to specify whatever encoding they want to use.

@MicahGale
Copy link
Collaborator Author

Thoughts, @tjlaboss?

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Nov 28, 2023, 09:20

Just Wing It ✔ is the Pythonic approach.

User-specifiable encoding would probably be an easy feature to implement. It should be an argument to the top-level read function, i.e., deck = read_input(file, encoding='mojibake'). Alternatively, a default could be set via a global variable. Where else in the public API would it appear?

@MicahGale
Copy link
Collaborator Author

This should only show up at the user level in montepy.read_input and MCNP_problem.write_to_input. I agree and like your function signatures.

Do you think this needs to be implemented prior to shipping 0.2.0 though? As that release is the first one where this explicit encoding will be shipped. I'm leaning towards no:

  1. 0.2.0 is taking so long.
  2. The user really shouldn't be using anything beyond vanilla ASCII anyways when the encoding is undocumented.
  3. I am a monster and hate emojis and all things pure in this world.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Nov 28, 2023, 11:48

No, I don't think this is a high-priority item.

@MicahGale
Copy link
Collaborator Author

changed time estimate to 1h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request An issue that improves the user interface. good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant