Skip to content

[change] Adjust settings to use new bw2data.projects method#8

Closed
dgdekoning wants to merge 2 commits intocauldron:bw25from
dgdekoning:feature/3-bw-pathing
Closed

[change] Adjust settings to use new bw2data.projects method#8
dgdekoning wants to merge 2 commits intocauldron:bw25from
dgdekoning:feature/3-bw-pathing

Conversation

@dgdekoning
Copy link
Copy Markdown

@dgdekoning dgdekoning commented Oct 25, 2023

This MR is related to #3.

The wasm branch in bw2data contains the new method that handles changing the brightway data directory.

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Update the documentation, please follow the numpy style guide
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, feature, ui, change, documentation, breaking, ci
    as they show up in the changelog
  • Link this PR to related issues.

@dgdekoning
Copy link
Copy Markdown
Author

dgdekoning commented Oct 25, 2023

Included in this MR are:

  • Changes to the code to make use of the new method (part 1 of the issue)
  • Changes to settings.py to replace the existing method with direct calls to bw2data for the _base_data_dir (part 2 of the issue).
  • Additional type-hinting and doc-string updates.
  • Changing from os use to pathlib.Path, except for one instance where os.path.normpath is used as there is no functionally similar call in pathlib.

Haven't worked on this in quite a while
@dgdekoning dgdekoning changed the title Draft: Adjust settings to use new bw2data.projects method [change] Adjust settings to use new bw2data.projects method Oct 27, 2023
self.data_dir = directory
self.filename = filename or "default_settings.json"
self.settings_file = os.path.join(self.data_dir, self.filename)
self.data_dir: str = directory
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should this also be an instance of pathlib.Path? We found it really convenient to have everything in bw2data be a Path, but maybe that is more trouble than it is worth here.

"""Update old settings, backwards compatibility method.

This function is only required for compatibility with the old settings file
and can be removed in a future release.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Any idea how old the old settings file is? Maybe we can remove this method now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It looks like the change it at least 4 years ago, so it should be fine to remove yes.

"""Returns the custom brightway directory, or the default"""
return self.settings.get("custom_bw_dirs", self.get_default_directory())
"""Returns the custom brightway directory, or the default."""
return self.settings.get("custom_bw_dirs") or str(bw2data.projects._base_data_dir)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is an important point, and I think we should document it more clearly. AFAICT there is a "default" directory set at startup, and this is what we can revert to at anytime. We can also change the data directory, but the only history of these changes that is made is that we remember the "default" directory.

NOTE: discards the biosphere3 database based on name.
"""
iterator = self.settings.get("read-only-databases", {}).items()
return (name for name, ro in iterator if not ro and name != "biosphere3")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need to make biosphere3 configurable, this will change in the future (there won't be just one string). We can either add it to the custom AB settings, or refer to bw2data.config.biosphere.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will have a look at the code for the bw2data implementation. Would prefer using the brightway standards.


def bw_projects(self, path: str):
"""Finds the bw_projects from the brightway2 environment provided by path"""
def bw_projects(self, path: str) -> List[str]:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you help me understand this? The RE does... not much, and why wouldn't we just read the projects from the projects.db database. There isn't any guarantee that each directory is a project.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So it looks like this bw_projects call was added after I stopped working on the AB and I can't explain why it was added here...

Will set up a separate 'unchanged' environment to see how it is being used.

@cmutel cmutel deleted the branch cauldron:bw25 June 10, 2024 09:09
@cmutel cmutel closed this Jun 10, 2024
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