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 method names from_file, load, to_file #67

Open
ehsteve opened this issue Apr 11, 2023 · 3 comments
Open

Rename method names from_file, load, to_file #67

ehsteve opened this issue Apr 11, 2023 · 3 comments

Comments

@ehsteve
Copy link
Member

ehsteve commented Apr 11, 2023

Currently, the method names are

  • load(filename) - to parse a binary file
  • from_file(filename) - to create a packet definition from a file (only csv right now)
  • to_file(filename) - to create a binary file with synthetic data (coming soon!)

I feel like all of these function names are unclear and require someone to read the documentation to figure out what they do which should not be the case.

The purpose of this issue is to discuss whether it is worthwhile to change these names and what the new names might be.

@ehsteve ehsteve changed the title Rename method name from_file Rename method names from_file, load, to_file Apr 11, 2023
@ehsteve
Copy link
Member Author

ehsteve commented Apr 11, 2023

One suggestion for new names

  • load() to parse()
  • from_file() to define_from_file()
  • to_file() no change?

@ddasilva
Copy link
Collaborator

We'll see what others think.. i think load() is as clear as parse() but I might be biased because I've stared at it so long. Changing from_file() to define_from_file() is definitely reasonable.

I do want to weigh these breaking API changes in terms of benefit to users vs annoyance to users. At the very least such changes should occur at major version changes. In general I think we should avoid breaking the API so much that we get ourselves a reputation where "upgrading ccsdspy means a lot of work."

@ehsteve
Copy link
Member Author

ehsteve commented Apr 11, 2023

Totally agree with you. May not be worth making some changes like for load. The reason I don't like load is that it could mean loading any file to do just about anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants