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

"id" vs "name" #13

Closed
klauer opened this issue Aug 7, 2020 · 9 comments
Closed

"id" vs "name" #13

klauer opened this issue Aug 7, 2020 · 9 comments

Comments

@klauer
Copy link
Contributor

klauer commented Aug 7, 2020

Is there a good reason we use a different "id" and device name here?

I'd expect to be able to do:

client = happi.Client.from_config()
client['cxi_dg1_slits']

But what actually works is:

client['CXI:DG1:JAWS']
-> SearchResult(client=<happi.client.Client object at 0x7f945f1adeb8>, metadata={'name': 'cxi_dg1_slits', 'device_class': 'pcdsdevices.device_types.Slits', 'args': ['{{prefix}}'], 'kwargs': {'name': '{{name}}'}, 'active': True, 'documentation': None, 'prefix': 'CXI:DG1:JAWS', 'beamline': 'CXI', 'location_group': None, 'functional_group': None, 'z': 1035.5, 'stand': 'DG1', 'detailed_screen': None, 'embedded_screen': None, 'engineering_screen': None, 'system': 'beam control', 'macros': None, 'lightpath': False, 'parent': None, '_id': 'CXI:DG1:JAWS', 'creation': 'Tue Feb 27 10:41:25 2018', 'last_edit': 'Thu Apr 12 14:40:08 2018', 'screen': None, 'type': 'pcdsdevices.happi.containers.Slits'})

This certainly does work, as it's explicitly using the name key:

client.find_device(name='cxi_dg1_slits')

Is this a happi issue, an issue with our convention, or my lack of understanding?

cc @ZLLentz @ZryletTC

@ZryletTC
Copy link
Contributor

ZryletTC commented Aug 7, 2020

It appears to be an issue with a way that we used to create happi entries. I've created a couple test devices and it seems nowadays the 'id' is set to the 'name', but looking through the database, there a ton of entries where it looks like the ids came from the devices' prefixes. We should probably transition those over...

@klauer
Copy link
Contributor Author

klauer commented Aug 7, 2020

OK, I think we should go through these and change it up. Will wait til Zach gets back, though, just in case something is relying on these IDs that we're not aware of.

@ZLLentz
Copy link
Member

ZLLentz commented Aug 10, 2020

Nothing is relying on the old ID formulation as far as I know. All applications I'm aware of are doing broader searches rather than using hard-coded names.

The original idea was that no two devices would ever have the same PV, thus it was the static, unique identifier. In practice it was easier to access devices by their shorter name fields.

@klauer
Copy link
Contributor Author

klauer commented Aug 11, 2020

This actually came up as a point of confusion from LBNL no more than a couple hours after I had submitted the issue.
Both NSLS-II and LBNL wanted to bypass the CLI and directly write JSON, interestingly enough.

That's a fair point about the PV names being unique; name clashes from hutch-to-hutch do seem possible. If we're all OK enforcing a globally-unique name for every device in the database, we should move forward with switching up the IDs.

@ZLLentz
Copy link
Member

ZLLentz commented Aug 11, 2020

I'm 👍 on switching the IDs. If you want to avoid name collisions, put your hutch in the name.

I'm not super surprised that people want to bypass the CLI and the IPython interface, but this isn't a good idea in general because the tools exist to help you create a working database. This implies that both of these interfaces need to be improved.

@ZryletTC
Copy link
Contributor

I'm 👍 on moving forward with the changes

@ZryletTC
Copy link
Contributor

And actually, it looks like we don't have any name conflicts :)

In [22]: with open('/Users/pennebak/Documents/GitHub/device_config/db.json') as infile: 
    ...:     db = json.load(infile) 
    ...:                                                                                            

In [23]: newdb = {}                                                                                 

In [24]: for key in db: 
    ...:     for newkey in newdb: 
    ...:         if db[key]['name'] == newdb[newkey]['name']: 
    ...:             print(f'{key} = {newkey}') 
    ...:         else: 
    ...:             #print(f'{key} is good') 
    ...:             pass 
    ...:     newdb[key] = db[key] 
    ...:                                                                                            

In [25]: db['XRT:M1H']['name']                                                                      
Out[25]: 'xrt_m1h'

@klauer
Copy link
Contributor Author

klauer commented Aug 14, 2020

Awesome!

@klauer
Copy link
Contributor Author

klauer commented Aug 14, 2020

Closed by #14

@klauer klauer closed this as completed Aug 14, 2020
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

No branches or pull requests

3 participants