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

Update CSVPointsSource #209

Merged
merged 11 commits into from
Jun 14, 2024
Merged

Update CSVPointsSource #209

merged 11 commits into from
Jun 14, 2024

Conversation

cmalinmayor
Copy link
Collaborator

@cmalinmayor cmalinmayor commented May 16, 2024

Pre-Merge Checklist:

  • if this PR adds a feature: request merge into the latest development branch (vX.Y-dev)
  • PR branch name is short and describes the feature/bug addressed in this PR
  • commit messages are consistent
  • changes reviewed by another contributor
  • tests cover changes
  • all tests pass

@cmalinmayor cmalinmayor marked this pull request as ready for review May 16, 2024 15:45
@cmalinmayor cmalinmayor requested a review from pattonw May 16, 2024 15:45
@cmalinmayor
Copy link
Collaborator Author

Note: the current black issue is on dev-v1.4

@cmalinmayor
Copy link
Collaborator Author

Closes #208

@cmalinmayor cmalinmayor linked an issue May 16, 2024 that may be closed by this pull request
# This fixture will run after seeds since it is set
# with autouse=True. So make sure to reset the seeds properly at the end
# of this fixture
random.seed(12345)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The seed setting seems a bit overkill. Looks like the only randomness is in the generation of the fake_points. So since the autouse=True seed fixture will run before either of the other two fixtures you can remove all of the seed setting in the other fixtures.

@pattonw pattonw merged commit d440f47 into dev-v1.4 Jun 14, 2024
6 of 7 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.

Use csv Reader in csv points source
2 participants