Skip to content

Explicitly request type when using nerc_rates #123

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

Merged
merged 1 commit into from
Apr 30, 2025

Conversation

KelvinLinBU
Copy link
Contributor

With the recent update to nerc_rates [1] which added type enforcement and requires users of nerc_rates to explicitly declare the requested type, our script has been updated to request the appropriate types. This should reduce ambiguity in the values we fetch from nerc_rates. Update requirements.txt to newest nerc-rates version.

[1] CCI-MOC/nerc-rates#22

@KelvinLinBU
Copy link
Contributor Author

@QuanMPhm

@QuanMPhm QuanMPhm requested review from QuanMPhm and naved001 April 30, 2025 16:43
requirements.txt Outdated
@@ -1,3 +1,4 @@
git+https://github.com/CCI-MOC/nerc-rates@33701ed#egg=nerc_rates
Copy link
Collaborator

@QuanMPhm QuanMPhm Apr 30, 2025

Choose a reason for hiding this comment

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

@naved001 Is it fine if we use a github url instead of the zip file?

@KelvinLinBU Based on the failed CI, the Docker image being used does not contain git, hence why we're getting nerc-rates through a zip file. Please use the zip file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And could you put the line in the same order. The diff becomes a bit confusing when things get shuffled around

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess just don't change this file at all?

@KelvinLinBU KelvinLinBU force-pushed the update-get-value-at branch 3 times, most recently from f862d9c to 86ebe3b Compare April 30, 2025 17:12
requirements.txt Outdated
@@ -1,3 +1,3 @@
requests>=2.18.4
boto3<1.36
https://github.com/CCI-MOC/nerc-rates/archive/main.zip
https://github.com/CCI-MOC/nerc-rates/archive/main.zip
Copy link
Collaborator

Choose a reason for hiding this comment

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

You removed a newline here?

With the recent update to `nerc_rates` [1] which added type enforcement and requires users of `nerc_rates` to explicitly declare the requested type, our script has been updated to request the appropriate types. This should reduce ambiguity in the values we fetch from nerc_rates

[1] CCI-MOC/nerc-rates#22
@KelvinLinBU KelvinLinBU force-pushed the update-get-value-at branch from 86ebe3b to d3bf75a Compare April 30, 2025 17:54
@QuanMPhm QuanMPhm merged commit 85a8c32 into CCI-MOC:main Apr 30, 2025
2 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.

3 participants