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

Allow relative target paths #110

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Conversation

thirtytwobits
Copy link
Member

Fix for issue #109. This relaxes some logic that required existing paths before creating a DSDLFile abstraction while adding a requirement that ReadableDSDLFile(s) are created with files that exist (i.e. it can't be "readable" if it doesn't exist). At the same time, we clarified (and fixed) the difference between the root_namespace_directories_or_names and lookup_directories arguments of read_files. Our new guidance is to dis-use lookup_directories unless there is a need to separate target lookup paths from dependent type look up paths. This was an unstated design goal that I forgot about and subsequently mis-implemented. This does change the behaviour of the API slightly which is why I've bumped the minor version instead of just the patch.

@thirtytwobits
Copy link
Member Author

ping?

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

Once the weird interface is adjusted it should be good to go. I'm sorry it's taking a long time to review.

pydsdl/_dsdl.py Outdated Show resolved Hide resolved
Fix for issue OpenCyphal#109. This relaxes some logic that required existing paths before creating a DSDLFile abstraction while adding a requirement that ReadableDSDLFile(s) are created with files that exist (i.e. it can't be "readable" if it doesn't exist). At the same time, we clarified (and fixed) the difference between the `root_namespace_directories_or_names` and `lookup_directories arguments` of read_files. Our new guidance is to dis-use `lookup_directories` unless there is a need to separate target lookup paths from dependent type look up paths. This was an unstated design goal that I forgot about and subsequently mis-implemented. This does change the behaviour of the API slightly which is why I've bumped the minor version instead of just the patch.
@thirtytwobits
Copy link
Member Author

No worries. I wasn't sure when you were going to be back is all. I'm trying to get Nunavut 3.0 into review by tomorrow and published next week.

Changes made. I forced pushed because the changes you requested were reverting changes I made so I wanted to have a diff that showed no superfluous changes remained.

@thirtytwobits thirtytwobits merged commit 3ea74de into OpenCyphal:master Jul 25, 2024
13 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.

2 participants