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

method to read conversion from csv #164

Merged
merged 16 commits into from
Oct 23, 2024
Merged

method to read conversion from csv #164

merged 16 commits into from
Oct 23, 2024

Conversation

crdanielbusch
Copy link
Contributor

@crdanielbusch crdanielbusch commented Oct 16, 2024

Pull request

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog snippet in a .rst file in the directory changelog_unreleased added – remember to start with a * to make it a bullet point

Description

Adds a public method that takes a csv file and returns a Conversion object. The user will then be able to generate a Conversion object with something like conversion = climate_categories.Conversion.from_csv("test.csv").

@crdanielbusch crdanielbusch marked this pull request as ready for review October 23, 2024 08:14
@crdanielbusch crdanielbusch self-assigned this Oct 23, 2024
Copy link
Member

@mikapfl mikapfl left a comment

Choose a reason for hiding this comment

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

Nice work. I've found a simplification, a bit too big to include it here in the review, so I made a pull request into your branch, see #165

@mikapfl
Copy link
Member

mikapfl commented Oct 23, 2024

Also, I noticed that pre-commit CI was not running any more due to the move to the primap-community org. I re-enabled it, so we should get authoritative linting again in the CI. 🙂

@crdanielbusch
Copy link
Contributor Author

I used the global ruff executable for code formatting in Pycharm, not sure which version. Now that we have the pre commit running again it should be the same format.

What are the guidelines for docstrings in PRIMAP2. The from_csv method just has a single line docstring at the moment. Other methods have the numpy stlye docstring with paramaters and output. I'm not sure which to use when.

@mikapfl
Copy link
Member

mikapfl commented Oct 23, 2024

Hi,

if you want to document parameters or need more space, use numpy style. If you think parameters don't need documentation and a single line is enough, just use a single line.

Cheers,

Mika

@mikapfl
Copy link
Member

mikapfl commented Oct 23, 2024

I think you have to merge main into this branch to make the pre-commit CI work automatically.

@crdanielbusch
Copy link
Contributor Author

I think you have to merge main into this branch to make the pre-commit CI work automatically.

Perfect, thanks! Then that's ready to merge

@mikapfl
Copy link
Member

mikapfl commented Oct 23, 2024

You should be able to merge yourself, right?

Daniel Busch and others added 3 commits October 23, 2024 14:02
@crdanielbusch
Copy link
Contributor Author

You should be able to merge yourself, right?

Yep. Just forgot the changelog

@crdanielbusch crdanielbusch merged commit 6a751d3 into main Oct 23, 2024
8 checks passed
@mikapfl mikapfl deleted the conversion-from-csv branch October 23, 2024 12:49
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.

2 participants