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

Second PR to update the Nomenclature documentation #242

Merged
merged 3 commits into from
May 5, 2023

Conversation

GretchenSchowalter
Copy link
Contributor

Hello! I took out what I thought were unnecessary methods listed in the documentation under API Documentation: CodeList and RegionProcessor (but let me know if I need to add back in some), and by doing this I was able to get rid of the table that had been there in previous versions.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Looks good, one minor suggestion inline

@@ -4,12 +4,12 @@
============

.. autoclass:: CodeList
:members:
:members: validate_items, from_directory, read_excel, to_yaml, to_pandas, to_csv, to_excel, codelist_repr, filter
Copy link
Member

Choose a reason for hiding this comment

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

Changing the order to a logical workflow (initialize, do, export) and removing an unnecessary method.

Suggested change
:members: validate_items, from_directory, read_excel, to_yaml, to_pandas, to_csv, to_excel, codelist_repr, filter
:members: from_directory, read_excel, validate_items, filter, to_yaml, to_pandas, to_csv, to_excel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes a lot of sense! Sounds good.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

Copy link
Contributor

@phackstock phackstock left a comment

Choose a reason for hiding this comment

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

Looks good to me as well thanks @GretchenSchowalter.

@phackstock phackstock merged commit 885a5f0 into IAMconsortium:main May 5, 2023
@GretchenSchowalter GretchenSchowalter deleted the UpdateDocs branch May 8, 2023 07:40
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