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

Refactor variable names for consistency #253

Merged
merged 3 commits into from
Jun 19, 2024

Conversation

Will-Tyler
Copy link
Contributor

Description

This pull request renames variables throughout the project for consistency and closes #165.

In general, the goal is to use the term "array" when referring to Zarr arrays and the term "field" in most other situations, as opposed to "column" or "variable".

In this pull request, I typically use the type of the Python object to determine an appropriate name. For example, I would name a Python variable that is a ZarrArraySpec instance array_spec. I determined the type of the objects by running the unit tests with the debugger. There are other locations in the code that refer to ZarrArraySpec instances as "fields" though—if desired, I can rename these as well for more consistency.

I also searched for uses of "var". In most cases, it seemed like "var" was used to mean "variant" and not "variable", so I left most of these alone.

Methods

I used regular expression searches to find the instances of the different terminology. For example, col[^umn] to find variables named something like col.

I used PyCharm's refactoring features to rename the Python variables.

Testing

I did not perform any testing of the changes. I am assuming that the unit tests will detect any issues.

Copy link
Contributor

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @Will-Tyler! This is a big improvement 👍

Good to merge after rebase (can get this in or #252 first, it's up to you).

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 98.841% (-0.04%) from 98.884%
when pulling 74fa91b on Will-Tyler:issue-165
into 31a5935 on sgkit-dev:main.

@Will-Tyler
Copy link
Contributor Author

#252 is merged now, and just rebased this branch!

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 98.843%. remained the same
when pulling 0dc6dea on Will-Tyler:issue-165
into d683a31 on sgkit-dev:main.

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 98.886% (+0.04%) from 98.843%
when pulling 0dc6dea on Will-Tyler:issue-165
into d683a31 on sgkit-dev:main.

@jeromekelleher jeromekelleher merged commit 20e6dd1 into sgkit-dev:main Jun 19, 2024
11 checks passed
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.

Refactoring: standardise on "field" in the code
3 participants